Skip to content

Commit

Permalink
url: enforce valid UTF-8 in WHATWG parser
Browse files Browse the repository at this point in the history 
This commit implements the Web IDL USVString conversion, which mandates
all unpaired Unicode surrogates be turned into U+FFFD REPLACEMENT
CHARACTER. It also disallows Symbols to be used as USVString per spec.

Certain functions call into C++ methods in the binding that use the
Utf8Value class to access string arguments. Utf8Value already does the
normalization using V8's String::Write, so in those cases, instead of
doing the full USVString normalization, only a symbol check is done
(`'' + val`, which uses ES's ToString, versus `String()` which has
special provisions for symbols).

PR-URL: #12507
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
@TimothyGu @evanlucas
TimothyGu authored and evanlucas committed May 1, 2017
1 parent e48e00b commit 7c9ca0f
Show file tree
Hide file tree
Showing 13 changed files with 509 additions and 44 deletions.
98 changes: 65 additions & 33 deletions lib/internal/url.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ const IteratorPrototype = Object.getPrototypeOf(
Object.getPrototypeOf([][Symbol.iterator]())
);

const unpairedSurrogateRe =
/([^\uD800-\uDBFF]|^)[\uDC00-\uDFFF]|[\uD800-\uDBFF](?![\uDC00-\uDFFF])/;
function toUSVString(val) {
const str = '' + val;
// As of V8 5.5, `str.search()` (and `unpairedSurrogateRe[@@search]()`) are
// slower than `unpairedSurrogateRe.exec()`.
const match = unpairedSurrogateRe.exec(str);
if (!match)
return str;
return binding.toUSVString(str, match.index);
}

class OpaqueOrigin {
toString() {
return 'null';
Expand Down Expand Up @@ -108,7 +120,6 @@ function onParseError(flags, input) {

// Reused by URL constructor and URL#href setter.
function parse(url, input, base) {
input = String(input);
const base_context = base ? base[context] : undefined;
url[context] = new StorageObject();
binding.parse(input.trim(), -1,
Expand Down Expand Up @@ -203,8 +214,10 @@ function onParseHashComplete(flags, protocol, username, password,

class URL {
constructor(input, base) {
// toUSVString is not needed.
input = '' + input;
if (base !== undefined && !(base instanceof URL))
base = new URL(String(base));
base = new URL(base);
parse(this, input, base);
}

Expand Down Expand Up @@ -312,6 +325,8 @@ Object.defineProperties(URL.prototype, {
return this[kFormat]({});
},
set(input) {
// toUSVString is not needed.
input = '' + input;
parse(this, input);
}
},
Expand All @@ -329,7 +344,8 @@ Object.defineProperties(URL.prototype, {
return this[context].scheme;
},
set(scheme) {
scheme = String(scheme);
// toUSVString is not needed.
scheme = '' + scheme;
if (scheme.length === 0)
return;
binding.parse(scheme, binding.kSchemeStart, null, this[context],
Expand All @@ -343,7 +359,8 @@ Object.defineProperties(URL.prototype, {
return this[context].username || '';
},
set(username) {
username = String(username);
// toUSVString is not needed.
username = '' + username;
if (!this.hostname)
return;
const ctx = this[context];
Expand All @@ -363,7 +380,8 @@ Object.defineProperties(URL.prototype, {
return this[context].password || '';
},
set(password) {
password = String(password);
// toUSVString is not needed.
password = '' + password;
if (!this.hostname)
return;
const ctx = this[context];
Expand All @@ -388,7 +406,8 @@ Object.defineProperties(URL.prototype, {
},
set(host) {
const ctx = this[context];
host = String(host);
// toUSVString is not needed.
host = '' + host;
if (this[cannotBeBase] ||
(this[special] && host.length === 0)) {
// Cannot set the host if cannot-be-base is set or
Expand All @@ -412,7 +431,8 @@ Object.defineProperties(URL.prototype, {
},
set(host) {
const ctx = this[context];
host = String(host);
// toUSVString is not needed.
host = '' + host;
if (this[cannotBeBase] ||
(this[special] && host.length === 0)) {
// Cannot set the host if cannot-be-base is set or
Expand All @@ -436,11 +456,12 @@ Object.defineProperties(URL.prototype, {
return port === undefined ? '' : String(port);
},
set(port) {
// toUSVString is not needed.
port = '' + port;
const ctx = this[context];
if (!ctx.host || this[cannotBeBase] ||
this.protocol === 'file:')
return;
port = String(port);
if (port === '') {
ctx.port = undefined;
return;
Expand All @@ -459,9 +480,11 @@ Object.defineProperties(URL.prototype, {
return ctx.path !== undefined ? `/${ctx.path.join('/')}` : '';
},
set(path) {
// toUSVString is not needed.
path = '' + path;
if (this[cannotBeBase])
return;
binding.parse(String(path), binding.kPathStart, null, this[context],
binding.parse(path, binding.kPathStart, null, this[context],
onParsePathComplete.bind(this));
}
},
Expand All @@ -474,7 +497,7 @@ Object.defineProperties(URL.prototype, {
},
set(search) {
const ctx = this[context];
search = String(search);
search = toUSVString(search);
if (!search) {
ctx.query = null;
ctx.flags &= ~binding.URL_FLAGS_HAS_QUERY;
Expand Down Expand Up @@ -506,7 +529,8 @@ Object.defineProperties(URL.prototype, {
},
set(hash) {
const ctx = this[context];
hash = String(hash);
// toUSVString is not needed.
hash = '' + hash;
if (this.protocol === 'javascript:')
return;
if (!hash) {
Expand Down Expand Up @@ -649,19 +673,22 @@ class URLSearchParams {
if (pair.length !== 2) {
throw new TypeError('Each query pair must be a name/value tuple');
}
this[searchParams].push(String(pair[0]), String(pair[1]));
const key = toUSVString(pair[0]);
const value = toUSVString(pair[1]);
this[searchParams].push(key, value);
}
} else {
// record<USVString, USVString>
this[searchParams] = [];
for (const key of Object.keys(init)) {
const value = String(init[key]);
for (var key of Object.keys(init)) {
key = toUSVString(key);
const value = toUSVString(init[key]);
this[searchParams].push(key, value);
}
}
} else {
// USVString
init = String(init);
init = toUSVString(init);
if (init[0] === '?') init = init.slice(1);
initSearchParams(this, init);
}
Expand Down Expand Up @@ -740,8 +767,8 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {
throw new TypeError('"name" and "value" arguments must be specified');
}

name = String(name);
value = String(value);
name = toUSVString(name);
value = toUSVString(value);
this[searchParams].push(name, value);
update(this[context], this);
},
Expand All @@ -755,7 +782,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {
}

const list = this[searchParams];
name = String(name);
name = toUSVString(name);
for (var i = 0; i < list.length;) {
const cur = list[i];
if (cur === name) {
Expand All @@ -776,7 +803,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {
}

const list = this[searchParams];
name = String(name);
name = toUSVString(name);
for (var i = 0; i < list.length; i += 2) {
if (list[i] === name) {
return list[i + 1];
Expand All @@ -795,7 +822,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {

const list = this[searchParams];
const values = [];
name = String(name);
name = toUSVString(name);
for (var i = 0; i < list.length; i += 2) {
if (list[i] === name) {
values.push(list[i + 1]);
Expand All @@ -813,7 +840,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {
}

const list = this[searchParams];
name = String(name);
name = toUSVString(name);
for (var i = 0; i < list.length; i += 2) {
if (list[i] === name) {
return true;
Expand All @@ -831,8 +858,8 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {
}

const list = this[searchParams];
name = String(name);
value = String(value);
name = toUSVString(name);
value = toUSVString(value);

// If there are any name-value pairs whose name is `name`, in `list`, set
// the value of the first such name-value pair to `value` and remove the
Expand Down Expand Up @@ -1094,11 +1121,13 @@ function originFor(url, base) {
}

function domainToASCII(domain) {
return binding.domainToASCII(String(domain));
// toUSVString is not needed.
return binding.domainToASCII('' + domain);
}

function domainToUnicode(domain) {
return binding.domainToUnicode(String(domain));
// toUSVString is not needed.
return binding.domainToUnicode('' + domain);
}

// Utility function that converts a URL object into an ordinary
Expand Down Expand Up @@ -1184,11 +1213,14 @@ function getPathFromURL(path) {
return isWindows ? getPathFromURLWin32(path) : getPathFromURLPosix(path);
}

exports.getPathFromURL = getPathFromURL;
exports.URL = URL;
exports.URLSearchParams = URLSearchParams;
exports.domainToASCII = domainToASCII;
exports.domainToUnicode = domainToUnicode;
exports.urlToOptions = urlToOptions;
exports.formatSymbol = kFormat;
exports.searchParamsSymbol = searchParams;
module.exports = {
toUSVString,
getPathFromURL,
URL,
URLSearchParams,
domainToASCII,
domainToUnicode,
urlToOptions,
formatSymbol: kFormat,
searchParamsSymbol: searchParams
};
53 changes: 53 additions & 0 deletions src/node_url.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include <unicode/utf.h>
#endif

#define UNICODE_REPLACEMENT_CHARACTER 0xFFFD

namespace node {

using v8::Array;
Expand Down Expand Up @@ -104,6 +106,21 @@ namespace url {
}
#endif

// If a UTF-16 character is a low/trailing surrogate.
static inline bool IsUnicodeTrail(uint16_t c) {
return (c & 0xFC00) == 0xDC00;
}

// If a UTF-16 character is a surrogate.
static inline bool IsUnicodeSurrogate(uint16_t c) {
return (c & 0xF800) == 0xD800;
}

// If a UTF-16 surrogate is a low/trailing one.
static inline bool IsUnicodeSurrogateTrail(uint16_t c) {
return (c & 0x400) != 0;
}

static url_host_type ParseIPv6Host(url_host* host,
const char* input,
size_t length) {
Expand Down Expand Up @@ -1356,6 +1373,41 @@ namespace url {
v8::NewStringType::kNormal).ToLocalChecked());
}

static void ToUSVString(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK_GE(args.Length(), 2);
CHECK(args[0]->IsString());
CHECK(args[1]->IsNumber());

TwoByteValue value(env->isolate(), args[0]);
const size_t n = value.length();

const int64_t start = args[1]->IntegerValue(env->context()).FromJust();
CHECK_GE(start, 0);

for (size_t i = start; i < n; i++) {
uint16_t c = value[i];
if (!IsUnicodeSurrogate(c)) {
continue;
} else if (IsUnicodeSurrogateTrail(c) || i == n - 1) {
value[i] = UNICODE_REPLACEMENT_CHARACTER;
} else {
uint16_t d = value[i + 1];
if (IsUnicodeTrail(d)) {
i++;
} else {
value[i] = UNICODE_REPLACEMENT_CHARACTER;
}
}
}

args.GetReturnValue().Set(
String::NewFromTwoByte(env->isolate(),
*value,
v8::NewStringType::kNormal,
n).ToLocalChecked());
}

static void DomainToASCII(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK_GE(args.Length(), 1);
Expand Down Expand Up @@ -1403,6 +1455,7 @@ namespace url {
Environment* env = Environment::GetCurrent(context);
env->SetMethod(target, "parse", Parse);
env->SetMethod(target, "encodeAuth", EncodeAuthSet);
env->SetMethod(target, "toUSVString", ToUSVString);
env->SetMethod(target, "domainToASCII", DomainToASCII);
env->SetMethod(target, "domainToUnicode", DomainToUnicode);

Expand Down
Loading

0 comments on commit 7c9ca0f

Please sign in to comment.