{"action":"created","issue":{"url":"https://api.github.com/repos/nodejs/node/issues/6357","repository_url":"https://api.github.com/repos/nodejs/node","labels_url":"https://api.github.com/repos/nodejs/node/issues/6357/labels{/name}","comments_url":"https://api.github.com/repos/nodejs/node/issues/6357/comments","events_url":"https://api.github.com/repos/nodejs/node/issues/6357/events","html_url":"https://github.com/nodejs/node/pull/6357","id":150592452,"number":6357,"title":"src: unify implementations of Utf8Value, TwoByteValue, etc.","user":{"login":"addaleax","id":899444,"avatar_url":"https://avatars.githubusercontent.com/u/899444?v=3","gravatar_id":"","url":"https://api.github.com/users/addaleax","html_url":"https://github.com/addaleax","followers_url":"https://api.github.com/users/addaleax/followers","following_url":"https://api.github.com/users/addaleax/following{/other_user}","gists_url":"https://api.github.com/users/addaleax/gists{/gist_id}","starred_url":"https://api.github.com/users/addaleax/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/addaleax/subscriptions","organizations_url":"https://api.github.com/users/addaleax/orgs","repos_url":"https://api.github.com/users/addaleax/repos","events_url":"https://api.github.com/users/addaleax/events{/privacy}","received_events_url":"https://api.github.com/users/addaleax/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/nodejs/node/labels/c++","name":"c++","color":"e11d21"},{"url":"https://api.github.com/repos/nodejs/node/labels/confirmed-bug","name":"confirmed-bug","color":"fc2929"},{"url":"https://api.github.com/repos/nodejs/node/labels/lts-watch-v4.x","name":"lts-watch-v4.x","color":"eb6420"}],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":17,"created_at":"2016-04-23T20:15:18Z","updated_at":"2016-06-02T00:10:51Z","closed_at":"2016-04-29T13:48:01Z","pull_request":{"url":"https://api.github.com/repos/nodejs/node/pulls/6357","html_url":"https://github.com/nodejs/node/pull/6357","diff_url":"https://github.com/nodejs/node/pull/6357.diff","patch_url":"https://github.com/nodejs/node/pull/6357.patch"},"body":"##### Checklist\r\n\r\n- [x] tests and code linting passes\r\n- [x] the commit message follows commit guidelines\r\n\r\n##### Affected core subsystem(s)\r\n\r\nsrc\r\n\r\n##### Description of change\r\n\r\nUnify the common code of `Utf8Value`, `TwoByteValue`, `BufferValue` and `StringBytes::InlineDecoder` into one class. Always make the result zero-terminated for the first three.\r\n\r\nThis fixes two problems in passing:\r\n\r\n* When the conversion of the input value to String fails, make the buffer zero-terminated anyway. Previously, this would have resulted in possibly reading uninitialized data in multiple places in the code. An instance of that problem can be reproduced by running e.g. `valgrind node -e 'net.isIP({ toString() { throw Error() } })'`. (Most other places where `Utf8Value` or `TwoByteValue` can possibly receive non-strings work too, there’s nothing special about `isIP`.)\r\n* Previously, `BufferValue` copied one byte too much from the source, possibly resulting in an out-of-bounds memory access. This can be reproduced by running e.g. `valgrind node -e 'fs.openSync(Buffer.from(\"node\".repeat(8192)), \"r\")'`.\r\n\r\nIf you don’t like the code changes by themselves because they don’t really modify behaviour in any way, I’d still like to see these issues fixed. Since I don’t see any reason why the latter one can’t occur in real life and bite users of the Buffer-accepting `fs` API in v6 with a segfault, I’m marking this with the 6.0.0 milestone.\r\n\r\n/cc @bnoordhuis @jasnell "},"comment":{"url":"https://api.github.com/repos/nodejs/node/issues/comments/223161789","html_url":"https://github.com/nodejs/node/pull/6357#issuecomment-223161789","issue_url":"https://api.github.com/repos/nodejs/node/issues/6357","id":223161789,"user":{"login":"addaleax","id":899444,"avatar_url":"https://avatars.githubusercontent.com/u/899444?v=3","gravatar_id":"","url":"https://api.github.com/users/addaleax","html_url":"https://github.com/addaleax","followers_url":"https://api.github.com/users/addaleax/followers","following_url":"https://api.github.com/users/addaleax/following{/other_user}","gists_url":"https://api.github.com/users/addaleax/gists{/gist_id}","starred_url":"https://api.github.com/users/addaleax/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/addaleax/subscriptions","organizations_url":"https://api.github.com/users/addaleax/orgs","repos_url":"https://api.github.com/users/addaleax/repos","events_url":"https://api.github.com/users/addaleax/events{/privacy}","received_events_url":"https://api.github.com/users/addaleax/received_events","type":"User","site_admin":false},"created_at":"2016-06-02T00:10:51Z","updated_at":"2016-06-02T00:10:51Z","body":"@TheAlphaNerd I think the change here depends on too much other stuff but I’ll make a backport PR with one of the bugfixes here."}}
{"actor":{"display_login":"addaleax"}}