The previous errors were incorrect here, as the code
only failed in situations where strings exceeded size limits or
an OOM situation was encountered, not for invalid encodings
(which aren’t even detected explicitly).
Unfortunately, these situations are hard to test for.
PR-URL: https://github.com/nodejs/node/pull/14579
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
- Return `MaybeLocal`s from `StringBytes::Encode`
- Add an `error` out parameter to pass JS exceptions to the callers
(instead of directly throwing)
- Simplify some of the string generation methods in `string_bytes.cc`
by unifying the `EXTERN_APEX` logic
- Reduce usage of deprecated V8 APIs.
- Remove error handling logic from JS, the `buffer.*Slice()` methods
now throw errors themselves.
- Left TODO comments for future semver-major error message
improvements.
This paves the way for better error messages coming out of the
StringBytes methods.
Ref: https://github.com/nodejs/node/issues/3175
PR-URL: https://github.com/nodejs/node/pull/12765
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Adds an optional second parameter to `node::Malloc()` and
an optional third parameter to `node::Realloc()` giving the
size/number of items to be allocated, in the style of `calloc(3)`.
Use a proper overflow check using division;
the previous `CHECK_GE(n * size, n);` would not detect all cases
of overflow (e.g. `size == SIZE_MAX / 2 && n == 3`).
PR-URL: https://github.com/nodejs/node/pull/8482
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
malloc(0) and realloc(ptr, 0) have implementation-defined behavior in
that the standard allows them to either return a unique pointer or a
nullptr for zero-sized allocation requests. Normalize by always using
a nullptr.
- Introduce node::malloc, node::realloc and node::calloc that should
be used throught our source.
- Update all existing node source files to use the new functions
instead of the native allocation functions.
Fixes: https://github.com/nodejs/node/issues/7549
PR-URL: https://github.com/nodejs/node/pull/7564
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
When node began using the OneByte API (f150d56) it also switched to
officially supporting ISO-8859-1. Though at the time no new encoding
string was introduced.
Introduce the new encoding string 'latin1' to be more explicit. The
previous 'binary' and documented as an alias to 'latin1'. While many
tests have switched to use 'latin1', there are still plenty that do both
'binary' and 'latin1' checks side-by-side to ensure there is no
regression.
PR-URL: https://github.com/nodejs/node/pull/7111
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Node already has support for base64 encoding and decoding that is not
visible outside the string_bytes.cc file. Our work on providing a
support for the Chrome inspector protocol
(https://github.com/nodejs/node/pull/6792) requires base64 encoding
support. Rather then introducing a second copy of the base64 encoder,
we suggest moving this code into a separate header.
PR-URL: https://github.com/nodejs/node/pull/6910
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
This makes several changes:
1. Allow path/filename to be passed in as a Buffer on fs methods
2. Add `options.encoding` to fs.readdir, fs.readdirSync, fs.readlink,
fs.readlinkSync and fs.watch.
3. Documentation updates
For 1... it's now possible to do:
```js
fs.open(Buffer('/fs/foo/bar'), 'w+', (err, fd) => { });
```
For 2...
```js
fs.readdir('/fs/foo/bar', {encoding:'hex'}, (err,list) => { });
fs.readdir('/fs/foo/bar', {encoding:'buffer'}, (err, list) => { });
```
encoding can also be passed as a string
```js
fs.readdir('/fs/foo/bar', 'hex', (err,list) => { });
```
The default encoding is set to UTF8 so this addresses the
discrepency that existed previously between fs.readdir and
fs.watch handling filenames differently.
Fixes: https://github.com/nodejs/node/issues/2088
Refs: https://github.com/nodejs/node/issues/3519
PR-URL: https://github.com/nodejs/node/pull/5616
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Versions of Node.js after v0.12 have relocated byte-swapping away from
the StringBytes::Encode function, thereby causing a nan test (which
accesses this function directly) to fail on big-endian machines.
This change re-introduces byte swapping in StringBytes::Encode,
done via a call to a function in util-inl. Another change in
NodeBuffer::StringSlice was necessary to avoid double byte swapping
in big-endian function calls to StringSlice.
PR-URL: https://github.com/nodejs/node/pull/3410
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Address comments and deprecations left in source files. These changes
include:
* Remove the deprecated API.
* Change Buffer::New() that did a copy of the data to Buffer::Copy()
* Change Buffer::Use() to Buffer::New()
PR-URL: https://github.com/nodejs/io.js/pull/1825
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Instead of aborting in case of internal failure, return an empty
Local<Object>. Using the MaybeLocal<T> API, users must check their
return values.
PR-URL: https://github.com/nodejs/io.js/pull/1825
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Make the inner loop execute fewer compare-and-branch executions per
processed byte, resulting in a 50% or more speedup.
This coincidentally fixes an out-of-bounds read:
while (unbase64(*src) < 0 && src < srcEnd)
Should have read:
while (src < srcEnd && unbase64(*src) < 0)
But this commit removes the offending code altogether.
Fixes: https://github.com/nodejs/io.js/issues/2166
PR-URL: https://github.com/nodejs/io.js/pull/2193
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
StringBytes::Write() did a plain memcpy() when is_extern is true but
that's wrong when the source is a two-byte string and the destination
a one-byte or UTF-8 string.
The impact is limited to strings > 1,031,913 bytes because those are
normally the only strings that are externalized, although the use of
the 'externalize strings' extension (--expose_externalize_string) can
also trigger it.
This commit also cleans up the bytes versus characters confusion in
StringBytes::Write() because that was closely intertwined with the
UCS-2 encoding regression. One wasn't fixable without the other.
Fixes: https://github.com/iojs/io.js/issues/1024
Fixes: https://github.com/joyent/node/issues/8683
PR-URL: https://github.com/iojs/io.js/pull/1042
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Make StringBytes::GetExternalParts() return the byte length for two-byte
strings, not the character length. Its callers operate on bytes, not
characters.
This also fixes StringBytes::Size() reporting only half of the actual
number of bytes for external two-byte strings.
PR-URL: https://github.com/iojs/io.js/pull/1042
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Introduced in joyent/node v0.10 as a backwards compatibility measure.
It's an ugly hack and allowing invalid UTF-8 is not a good idea in the
first place, remove it.
PR-URL: https://github.com/iojs/io.js/pull/1042
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Large external two-byte strings reported their character length instead
of their byte length, throwing off the garbage collector heuristic by
a factor of two.
PR-URL: https://github.com/iojs/io.js/pull/1042
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The copyright and license notice is already in the LICENSE file. There
is no justifiable reason to also require that it be included in every
file, since the individual files are not individually distributed except
as part of the entire package.
Move the big endian to little endian conversion logic for UCS2 input
from src/string_bytes.cc to src/node_buffer.cc; StringSlice() is the
only function that actually needs it and with this commit, a second
copy is avoided on big endian architectures.
Introduce two-byte overloads of node::Encode() and StringBytes::Encode()
that ensure that the input is suitably aligned.
Revisits commit 535fec8 from yesterday.
Seen with g++ 4.9.2 on x86_64 Linux: a SIGSEGV is generated when the
input to v8::String::NewFromTwoByte() is not suitably aligned.
g++ 4.9.2 emits SSE instructions for copy loops. That requires aligned
input but that was something StringBytes::Encode() did not enforce until
now. Make a properly aligned copy before handing off the input to V8.
We could, as an optimization, check that the pointer is aligned on a
two-byte boundary but that is technically still UB; pointers-to-char
are allowed to alias other pointers but the reverse is not true:
a pointer-to-uint16_t that aliases a pointer-to-char is in violation
of the pointer aliasing rules.
See https://code.google.com/p/v8/issues/detail?id=3694
Fixes segfaulting test simple/test-stream2-writable.
PR-URL: https://github.com/iojs/io.js/pull/127
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
* v8::Platform has a new MonotonicallyIncreasingTime() method,
implement it.
* The ASCII apocalypse continues with the replacement of external
ASCII strings with external one byte strings.
The previous commits fixed oversights in destructors that should have
been marked virtual but weren't. This commit marks destructors from
derived classes with the override keyword.
Now that we are building with C++11 features enabled, replace use
of NULL with nullptr.
The benefit of using nullptr is that it can never be confused for
an integral type because it does not support implicit conversions
to integral types except boolean - unlike NULL, which is defined
as a literal `0`.
Mechanically replace assert() statements with UNREACHABLE(), CHECK(),
or CHECK_{EQ,NE,LT,GT,LE,GE}() statements.
The exceptions are src/node.h and src/node_object_wrap.h because they
are public headers.
PR-URL: https://github.com/node-forward/node/pull/16
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Previously v8's WriteUtf8 function would produce invalid utf-8 output
when encountering unmatched surrogate code units [1]. The new
REPLACE_INVALID_UTF8 option fixes that by replacing invalid code points
with the unicode replacement character.
[1]: JS Strings are defined as arrays of 16 bit unsigned integers. There
is no unicode enforcement, so one can easily end up with invalid unicode
code unit sequences inside a string.
64bit constants are keyed for x64 platforms only, add PowerPC based
platform constants.
Node's "ucs2" encoding wants LE character data stored in the Buffer, so
we need to reorder on BE platforms. See
http://nodejs.org/api/buffer.html regarding Node's "ucs2" encoding
specification
Signed-off-by: Timothy J Fontaine <tjfontaine@gmail.com>
Don't call DecodeWrite() with a Buffer as its argument because it in
turn calls StringBytes::Write() and that method expects a Local<String>.
"Why then does that function take a Local<Value>?" I hear you ask.
Good question but I don't have the answer. I added a CHECK for good
measure and what do you know, all of a sudden a large number of crypto
tests started failing.
Calling DecodeWrite(BINARY) on a buffer is nonsensical anyway: if you
want the contents of the buffer, just copy out the data, there is no
need to decode it - and that's exactly what this commit does.
Fixes a great many instances of the following run-time error in debug
builds:
FATAL ERROR: v8::String::Cast() Could not convert to string
Make calls to v8::Isolate::AdjustAmountOfExternalAllocatedMemory() take
special care when negating 32 bits unsigned types like size_t.
Before this commit, values were negated before they got promoted to
64 bits, meaning that on 32 bits architectures, a value like 42 got
cast to 4294967254 instead of -42.
That in turn made the garbage collector start scavenging like crazy
because it thought the system was out of memory.
That's bad enough but calls to AdjustAmountOfExternalAllocatedMemory()
were made from weak callbacks, i.e. at a time when the garbage collector
was already busy. It triggered asserts in debug builds and caused
random crashes and memory corruption in release builds.
The behavior in release builds is arguably a V8 bug and should perhaps
be reported upstream.
Partially fixes#7309 but requires further bug fixes to src/smalloc.cc
that I'll address in a follow-up commit.
Anna Henningsen