-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Fix joyent/node#8683 #8744
Fix joyent/node#8683 #8744
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,12 +119,24 @@ size_t Length(Handle<Object> obj) { | |
|
||
Local<Object> New(Isolate* isolate, Handle<String> string, enum encoding enc) { | ||
EscapableHandleScope scope(isolate); | ||
|
||
size_t length = StringBytes::Size(isolate, string, enc); | ||
Local<String> tmp_string; | ||
|
||
if (string->IsExternalAscii()) { | ||
const String::ExternalAsciiStringResource* ext; | ||
ext = string->GetExternalAsciiStringResource(); | ||
tmp_string = String::NewFromOneByte(isolate, reinterpret_cast<const uint8_t*>(ext->data()), String::NewStringType::kNormalString, ext->length()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. |
||
} else if (string->IsExternal()) { | ||
const String::ExternalStringResource* ext; | ||
ext = string->GetExternalStringResource(); | ||
tmp_string = String::NewFromTwoByte(isolate, ext->data(), String::NewStringType::kNormalString, ext->length()); | ||
} else { | ||
tmp_string = string; | ||
} | ||
size_t length = StringBytes::Size(isolate, tmp_string, enc); | ||
|
||
Local<Object> buf = New(length); | ||
char* data = Buffer::Data(buf); | ||
StringBytes::Write(isolate, data, length, string, enc); | ||
StringBytes::Write(isolate, data, length, tmp_string, enc); | ||
|
||
return scope.Escape(buf); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -304,6 +304,14 @@ size_t StringBytes::Write(Isolate* isolate, | |
|
||
CHECK(val->IsString() == true); | ||
Local<String> str = val.As<String>(); | ||
bool is_extern_ascii = is_extern && str->IsExternalAscii(); | ||
if (is_extern) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove this. |
||
if (is_extern_ascii) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
str = String::NewFromOneByte(isolate, reinterpret_cast<const uint8_t*>(data), String::NewStringType::kNormalString, len); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this pass |
||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
str = String::NewFromTwoByte(isolate, reinterpret_cast<const uint16_t*>(data), String::NewStringType::kNormalString, len); | ||
} | ||
} | ||
len = len < buflen ? len : buflen; | ||
|
||
int flags = String::NO_NULL_TERMINATION | | ||
|
@@ -313,31 +321,20 @@ size_t StringBytes::Write(Isolate* isolate, | |
case ASCII: | ||
case BINARY: | ||
case BUFFER: | ||
if (is_extern) | ||
memcpy(buf, data, len); | ||
else | ||
len = str->WriteOneByte(reinterpret_cast<uint8_t*>(buf), | ||
0, | ||
buflen, | ||
flags); | ||
len = str->WriteOneByte(reinterpret_cast<uint8_t*>(buf), | ||
0, | ||
buflen, | ||
flags); | ||
if (chars_written != NULL) | ||
*chars_written = len; | ||
break; | ||
|
||
case UTF8: | ||
if (is_extern) | ||
// TODO(tjfontaine) should this validate invalid surrogate pairs as | ||
// well? | ||
memcpy(buf, data, len); | ||
else | ||
len = str->WriteUtf8(buf, buflen, chars_written, WRITE_UTF8_FLAGS); | ||
len = str->WriteUtf8(buf, buflen, chars_written, WRITE_UTF8_FLAGS); | ||
break; | ||
|
||
case UCS2: | ||
if (is_extern) | ||
memcpy(buf, data, len * 2); | ||
else | ||
len = str->Write(reinterpret_cast<uint16_t*>(buf), 0, buflen, flags); | ||
len = str->Write(reinterpret_cast<uint16_t*>(buf), 0, buflen, flags); | ||
if (IsBigEndian()) { | ||
// Node's "ucs2" encoding wants LE character data stored in | ||
// the Buffer, so we need to reorder on BE platforms. See | ||
|
@@ -354,24 +351,20 @@ size_t StringBytes::Write(Isolate* isolate, | |
break; | ||
|
||
case BASE64: | ||
if (is_extern) { | ||
len = base64_decode(buf, buflen, data, extlen); | ||
} else { | ||
do { | ||
String::Value value(str); | ||
len = base64_decode(buf, buflen, *value, value.length()); | ||
} | ||
} while (0); | ||
if (chars_written != NULL) { | ||
*chars_written = len; | ||
} | ||
break; | ||
|
||
case HEX: | ||
if (is_extern) { | ||
len = hex_decode(buf, buflen, data, extlen); | ||
} else { | ||
do { | ||
String::Value value(str); | ||
len = hex_decode(buf, buflen, *value, value.length()); | ||
} | ||
} while (0); | ||
if (chars_written != NULL) { | ||
*chars_written = len * 2; | ||
} | ||
|
@@ -459,10 +452,17 @@ size_t StringBytes::Size(Isolate* isolate, | |
return Buffer::Length(val); | ||
|
||
const char* data; | ||
if (GetExternalParts(isolate, val, &data, &data_size)) | ||
return data_size; | ||
bool is_extern = GetExternalParts(isolate, val, &data, &data_size); | ||
|
||
Local<String> str = val->ToString(); | ||
bool is_extern_ascii = is_extern && str->IsExternalAscii(); | ||
if (is_extern) { | ||
if (is_extern_ascii) { | ||
str = String::NewFromOneByte(isolate, reinterpret_cast<const uint8_t*>(data), String::NewStringType::kNormalString, data_size); | ||
} else { | ||
str = String::NewFromTwoByte(isolate, reinterpret_cast<const uint16_t*>(data), String::NewStringType::kNormalString, data_size); | ||
} | ||
} | ||
|
||
switch (encoding) { | ||
case BINARY: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
// Copyright Joyent, Inc. and other Node contributors. | ||
// | ||
// Permission is hereby granted, free of charge, to any person obtaining a | ||
// copy of this software and associated documentation files (the | ||
// "Software"), to deal in the Software without restriction, including | ||
// without limitation the rights to use, copy, modify, merge, publish, | ||
// distribute, sublicense, and/or sell copies of the Software, and to permit | ||
// persons to whom the Software is furnished to do so, subject to the | ||
// following conditions: | ||
// | ||
// The above copyright notice and this permission notice shall be included | ||
// in all copies or substantial portions of the Software. | ||
// | ||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS | ||
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF | ||
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN | ||
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, | ||
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR | ||
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE | ||
// USE OR OTHER DEALINGS IN THE SOFTWARE. | ||
|
||
// Flags: --expose_externalize_string | ||
|
||
var common = require('../common'); | ||
var assert = require('assert'); | ||
|
||
assert(typeof externalizeString === 'function', 'Run this test with --expose_externalize_string.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 80 column limit. |
||
|
||
var str = '中'; | ||
var buf = new Buffer(str); | ||
assert.equal(buf.toString('hex'), 'e4b8ad'); | ||
|
||
externalizeString(str, true); // force two bytes string | ||
var extbuf = new Buffer(str); // convert string to utf8 buffer | ||
assert.equal(extbuf.toString('hex'), 'e4b8ad'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing newline character. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading #8683 correctly the issue is that the length computation is incorrect. Creating a temporary string doesn't seem necessary at all. Why not just the length computation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how to compute the length correctly for each encoding. But creating a temporary string is the simplest way to fix this issue.