src: general C++ cleanup in node_url.cc

- Merge `domain` and `opaque` storage in URL parser:

  This just simplifies the code a bit, having multiple fields
  in an union with the same type is usually just overhead.

- Add move variant of `URLHost::ToString()`:

  This helps avoid unnecessary string copy operations, especially
  since we control the lifetime of `URLHost` objects pretty well.

- Use const refs in node_url.cc where appropriate

- Remove or reduce overly generous `.reserve()` calls:

  These would otherwise keep a lot of unused memory lying around.

- Return return values instead of unnecessary pointer arguments

- Use more common/expressive variable names

- Avoid macro use, reduce number of unnecessary JS strings:

  There’s no reason for `GET`, `GET_AND_SET` and `UTF8STRING` to be
  macros. Also, `GET` would previously create a JS string instance
  for each single call, even though the strings it was called
  with were compile-time constants.

- Avoid unnecessary JS casts when the type of a value is known

- Avoid (commonly unnecessary) copy for whitespace stripping

PR-URL: https://github.com/nodejs/node/pull/19598
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This commit is contained in:
Anna Henningsen 2018-03-25 15:02:23 +02:00
parent b7cfd278a5
commit ae70e2bc34
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9
2 changed files with 190 additions and 149 deletions

View File

@ -165,11 +165,13 @@ struct PackageConfig {
V(fingerprint_string, "fingerprint") \ V(fingerprint_string, "fingerprint") \
V(fingerprint256_string, "fingerprint256") \ V(fingerprint256_string, "fingerprint256") \
V(flags_string, "flags") \ V(flags_string, "flags") \
V(fragment_string, "fragment") \
V(get_data_clone_error_string, "_getDataCloneError") \ V(get_data_clone_error_string, "_getDataCloneError") \
V(get_shared_array_buffer_id_string, "_getSharedArrayBufferId") \ V(get_shared_array_buffer_id_string, "_getSharedArrayBufferId") \
V(gid_string, "gid") \ V(gid_string, "gid") \
V(handle_string, "handle") \ V(handle_string, "handle") \
V(homedir_string, "homedir") \ V(homedir_string, "homedir") \
V(host_string, "host") \
V(hostmaster_string, "hostmaster") \ V(hostmaster_string, "hostmaster") \
V(ignore_string, "ignore") \ V(ignore_string, "ignore") \
V(infoaccess_string, "infoAccess") \ V(infoaccess_string, "infoAccess") \
@ -226,6 +228,7 @@ struct PackageConfig {
V(order_string, "order") \ V(order_string, "order") \
V(owner_string, "owner") \ V(owner_string, "owner") \
V(parse_error_string, "Parse Error") \ V(parse_error_string, "Parse Error") \
V(password_string, "password") \
V(path_string, "path") \ V(path_string, "path") \
V(pending_handle_string, "pendingHandle") \ V(pending_handle_string, "pendingHandle") \
V(pbkdf2_error_string, "PBKDF2 Error") \ V(pbkdf2_error_string, "PBKDF2 Error") \
@ -239,6 +242,7 @@ struct PackageConfig {
V(produce_cached_data_string, "produceCachedData") \ V(produce_cached_data_string, "produceCachedData") \
V(promise_string, "promise") \ V(promise_string, "promise") \
V(pubkey_string, "pubkey") \ V(pubkey_string, "pubkey") \
V(query_string, "query") \
V(raw_string, "raw") \ V(raw_string, "raw") \
V(read_host_object_string, "_readHostObject") \ V(read_host_object_string, "_readHostObject") \
V(readable_string, "readable") \ V(readable_string, "readable") \
@ -247,6 +251,7 @@ struct PackageConfig {
V(rename_string, "rename") \ V(rename_string, "rename") \
V(replacement_string, "replacement") \ V(replacement_string, "replacement") \
V(retry_string, "retry") \ V(retry_string, "retry") \
V(scheme_string, "scheme") \
V(serial_string, "serial") \ V(serial_string, "serial") \
V(scopeid_string, "scopeid") \ V(scopeid_string, "scopeid") \
V(serial_number_string, "serialNumber") \ V(serial_number_string, "serialNumber") \

View File

@ -15,6 +15,7 @@ using v8::Context;
using v8::Function; using v8::Function;
using v8::FunctionCallbackInfo; using v8::FunctionCallbackInfo;
using v8::HandleScope; using v8::HandleScope;
using v8::Int32;
using v8::Integer; using v8::Integer;
using v8::Isolate; using v8::Isolate;
using v8::Local; using v8::Local;
@ -26,24 +27,13 @@ using v8::TryCatch;
using v8::Undefined; using v8::Undefined;
using v8::Value; using v8::Value;
#define GET(env, obj, name) \ inline Local<String> Utf8String(Isolate* isolate, const std::string& str) {
obj->Get(env->context(), \ return String::NewFromUtf8(isolate,
OneByteString(env->isolate(), name)).ToLocalChecked() str.data(),
v8::NewStringType::kNormal,
#define GET_AND_SET(env, obj, name, data, flag) \ str.length()).ToLocalChecked();
{ \
Local<Value> val = GET(env, obj, #name); \
if (val->IsString()) { \
Utf8Value value(env->isolate(), val.As<String>()); \
data->name = *value; \
data->flags |= flag; \
} \
} }
#define UTF8STRING(isolate, str) \
String::NewFromUtf8(isolate, str.c_str(), v8::NewStringType::kNormal) \
.ToLocalChecked()
namespace url { namespace url {
namespace { namespace {
@ -69,6 +59,8 @@ class URLHost {
inline bool ParsingFailed() const { return type_ == HostType::H_FAILED; } inline bool ParsingFailed() const { return type_ == HostType::H_FAILED; }
std::string ToString() const; std::string ToString() const;
// Like ToString(), but avoids a copy in exchange for invalidating `*this`.
std::string ToStringMove();
private: private:
enum class HostType { enum class HostType {
@ -80,10 +72,9 @@ class URLHost {
}; };
union Value { union Value {
std::string domain; std::string domain_or_opaque;
uint32_t ipv4; uint32_t ipv4;
uint16_t ipv6[8]; uint16_t ipv6[8];
std::string opaque;
~Value() {} ~Value() {}
Value() : ipv4(0) {} Value() : ipv4(0) {}
@ -95,9 +86,12 @@ class URLHost {
inline void Reset() { inline void Reset() {
using string = std::string; using string = std::string;
switch (type_) { switch (type_) {
case HostType::H_DOMAIN: value_.domain.~string(); break; case HostType::H_DOMAIN:
case HostType::H_OPAQUE: value_.opaque.~string(); break; case HostType::H_OPAQUE:
default: break; value_.domain_or_opaque.~string();
break;
default:
break;
} }
type_ = HostType::H_FAILED; type_ = HostType::H_FAILED;
} }
@ -113,13 +107,13 @@ class URLHost {
inline void SetOpaque(std::string&& string) { inline void SetOpaque(std::string&& string) {
Reset(); Reset();
type_ = HostType::H_OPAQUE; type_ = HostType::H_OPAQUE;
new(&value_.opaque) std::string(std::move(string)); new(&value_.domain_or_opaque) std::string(std::move(string));
} }
inline void SetDomain(std::string&& string) { inline void SetDomain(std::string&& string) {
Reset(); Reset();
type_ = HostType::H_DOMAIN; type_ = HostType::H_DOMAIN;
new(&value_.domain) std::string(std::move(string)); new(&value_.domain_or_opaque) std::string(std::move(string));
} }
}; };
@ -136,7 +130,8 @@ URLHost::~URLHost() {
XX(ARG_PORT) \ XX(ARG_PORT) \
XX(ARG_PATH) \ XX(ARG_PATH) \
XX(ARG_QUERY) \ XX(ARG_QUERY) \
XX(ARG_FRAGMENT) XX(ARG_FRAGMENT) \
XX(ARG_COUNT) // This one has to be last.
#define ERR_ARGS(XX) \ #define ERR_ARGS(XX) \
XX(ERR_ARG_FLAGS) \ XX(ERR_ARG_FLAGS) \
@ -665,7 +660,7 @@ inline std::string PercentDecode(const char* input, size_t len) {
XX("ws:", 80) \ XX("ws:", 80) \
XX("wss:", 443) XX("wss:", 443)
inline bool IsSpecial(std::string scheme) { inline bool IsSpecial(const std::string& scheme) {
#define XX(name, _) if (scheme == name) return true; #define XX(name, _) if (scheme == name) return true;
SPECIALS(XX); SPECIALS(XX);
#undef XX #undef XX
@ -684,7 +679,7 @@ inline bool StartsWithWindowsDriveLetter(const char* p, const char* end) {
p[2] == '#'); p[2] == '#');
} }
inline int NormalizePort(std::string scheme, int p) { inline int NormalizePort(const std::string& scheme, int p) {
#define XX(name, port) if (scheme == name && p == port) return -1; #define XX(name, port) if (scheme == name && p == port) return -1;
SPECIALS(XX); SPECIALS(XX);
#undef XX #undef XX
@ -930,7 +925,7 @@ void URLHost::ParseIPv4Host(const char* input, size_t length, bool* is_ipv4) {
void URLHost::ParseOpaqueHost(const char* input, size_t length) { void URLHost::ParseOpaqueHost(const char* input, size_t length) {
CHECK_EQ(type_, HostType::H_FAILED); CHECK_EQ(type_, HostType::H_FAILED);
std::string output; std::string output;
output.reserve(length * 3); output.reserve(length);
for (size_t i = 0; i < length; i++) { for (size_t i = 0; i < length; i++) {
const char ch = input[i]; const char ch = input[i];
if (ch != '%' && IsForbiddenHostCodePoint(ch)) { if (ch != '%' && IsForbiddenHostCodePoint(ch)) {
@ -1022,14 +1017,27 @@ inline T* FindLongestZeroSequence(T* values, size_t len) {
return result; return result;
} }
std::string URLHost::ToStringMove() {
std::string return_value;
switch (type_) {
case HostType::H_DOMAIN:
case HostType::H_OPAQUE:
return_value = std::move(value_.domain_or_opaque);
break;
default:
return_value = ToString();
break;
}
Reset();
return return_value;
}
std::string URLHost::ToString() const { std::string URLHost::ToString() const {
std::string dest; std::string dest;
switch (type_) { switch (type_) {
case HostType::H_DOMAIN: case HostType::H_DOMAIN:
return value_.domain;
break;
case HostType::H_OPAQUE: case HostType::H_OPAQUE:
return value_.opaque; return value_.domain_or_opaque;
break; break;
case HostType::H_IPV4: { case HostType::H_IPV4: {
dest.reserve(15); dest.reserve(15);
@ -1089,103 +1097,125 @@ bool ParseHost(const std::string& input,
host.ParseHost(input.c_str(), input.length(), is_special, unicode); host.ParseHost(input.c_str(), input.length(), is_special, unicode);
if (host.ParsingFailed()) if (host.ParsingFailed())
return false; return false;
*output = host.ToString(); *output = host.ToStringMove();
return true; return true;
} }
inline void Copy(Environment* env, inline std::vector<std::string> FromJSStringArray(Environment* env,
Local<Array> ary, Local<Array> array) {
std::vector<std::string>* vec) { std::vector<std::string> vec;
const int32_t len = ary->Length(); const int32_t len = array->Length();
if (len == 0) if (len == 0)
return; // nothing to copy return vec; // nothing to copy
vec->reserve(len); vec.reserve(len);
for (int32_t n = 0; n < len; n++) { for (int32_t n = 0; n < len; n++) {
Local<Value> val = ary->Get(env->context(), n).ToLocalChecked(); Local<Value> val = array->Get(env->context(), n).ToLocalChecked();
if (val->IsString()) { if (val->IsString()) {
Utf8Value value(env->isolate(), val.As<String>()); Utf8Value value(env->isolate(), val.As<String>());
vec->push_back(std::string(*value, value.length())); vec.emplace_back(*value, value.length());
} }
} }
return vec;
} }
inline Local<Array> Copy(Environment* env, inline Local<Array> ToJSStringArray(Environment* env,
const std::vector<std::string>& vec) { const std::vector<std::string>& vec) {
Isolate* isolate = env->isolate(); Isolate* isolate = env->isolate();
Local<Array> ary = Array::New(isolate, vec.size()); Local<Array> array = Array::New(isolate, vec.size());
for (size_t n = 0; n < vec.size(); n++) for (size_t n = 0; n < vec.size(); n++)
ary->Set(env->context(), n, UTF8STRING(isolate, vec[n])).FromJust(); array->Set(env->context(), n, Utf8String(isolate, vec[n])).FromJust();
return ary; return array;
} }
inline void HarvestBase(Environment* env, inline url_data HarvestBase(Environment* env, Local<Object> base_obj) {
struct url_data* base, url_data base;
Local<Object> base_obj) {
Local<Context> context = env->context(); Local<Context> context = env->context();
Local<Value> flags = GET(env, base_obj, "flags"); Local<Value> flags =
base_obj->Get(env->context(), env->flags_string()).ToLocalChecked();
if (flags->IsInt32()) if (flags->IsInt32())
base->flags = flags->Int32Value(context).FromJust(); base.flags = flags->Int32Value(context).FromJust();
Local<Value> scheme = GET(env, base_obj, "scheme"); Local<Value> scheme =
base->scheme = Utf8Value(env->isolate(), scheme).out(); base_obj->Get(env->context(), env->scheme_string()).ToLocalChecked();
base.scheme = Utf8Value(env->isolate(), scheme).out();
GET_AND_SET(env, base_obj, username, base, URL_FLAGS_HAS_USERNAME); auto GetStr = [&](std::string url_data::* member,
GET_AND_SET(env, base_obj, password, base, URL_FLAGS_HAS_PASSWORD); int flag,
GET_AND_SET(env, base_obj, host, base, URL_FLAGS_HAS_HOST); Local<String> name) {
GET_AND_SET(env, base_obj, query, base, URL_FLAGS_HAS_QUERY); Local<Value> value = base_obj->Get(env->context(), name).ToLocalChecked();
GET_AND_SET(env, base_obj, fragment, base, URL_FLAGS_HAS_FRAGMENT); if (value->IsString()) {
Local<Value> port = GET(env, base_obj, "port"); Utf8Value utf8value(env->isolate(), value.As<String>());
(base.*member).assign(*utf8value, utf8value.length());
base.flags |= flag;
}
};
GetStr(&url_data::username, URL_FLAGS_HAS_USERNAME, env->username_string());
GetStr(&url_data::password, URL_FLAGS_HAS_PASSWORD, env->password_string());
GetStr(&url_data::host, URL_FLAGS_HAS_HOST, env->host_string());
GetStr(&url_data::query, URL_FLAGS_HAS_QUERY, env->query_string());
GetStr(&url_data::fragment, URL_FLAGS_HAS_FRAGMENT, env->fragment_string());
Local<Value> port =
base_obj->Get(env->context(), env->port_string()).ToLocalChecked();
if (port->IsInt32()) if (port->IsInt32())
base->port = port->Int32Value(context).FromJust(); base.port = port.As<Int32>()->Value();
Local<Value> path = GET(env, base_obj, "path");
Local<Value>
path = base_obj->Get(env->context(), env->path_string()).ToLocalChecked();
if (path->IsArray()) { if (path->IsArray()) {
base->flags |= URL_FLAGS_HAS_PATH; base.flags |= URL_FLAGS_HAS_PATH;
Copy(env, path.As<Array>(), &(base->path)); base.path = FromJSStringArray(env, path.As<Array>());
} }
return base;
} }
inline void HarvestContext(Environment* env, inline url_data HarvestContext(Environment* env, Local<Object> context_obj) {
struct url_data* context, url_data context;
Local<Object> context_obj) { Local<Value> flags =
Local<Value> flags = GET(env, context_obj, "flags"); context_obj->Get(env->context(), env->flags_string()).ToLocalChecked();
if (flags->IsInt32()) { if (flags->IsInt32()) {
int32_t _flags = flags->Int32Value(env->context()).FromJust(); static const int32_t copy_flags_mask =
if (_flags & URL_FLAGS_SPECIAL) URL_FLAGS_SPECIAL |
context->flags |= URL_FLAGS_SPECIAL; URL_FLAGS_CANNOT_BE_BASE |
if (_flags & URL_FLAGS_CANNOT_BE_BASE) URL_FLAGS_HAS_USERNAME |
context->flags |= URL_FLAGS_CANNOT_BE_BASE; URL_FLAGS_HAS_PASSWORD |
if (_flags & URL_FLAGS_HAS_USERNAME) URL_FLAGS_HAS_HOST;
context->flags |= URL_FLAGS_HAS_USERNAME; context.flags |= flags.As<Int32>()->Value() & copy_flags_mask;
if (_flags & URL_FLAGS_HAS_PASSWORD)
context->flags |= URL_FLAGS_HAS_PASSWORD;
if (_flags & URL_FLAGS_HAS_HOST)
context->flags |= URL_FLAGS_HAS_HOST;
} }
Local<Value> scheme = GET(env, context_obj, "scheme"); Local<Value> scheme =
context_obj->Get(env->context(), env->scheme_string()).ToLocalChecked();
if (scheme->IsString()) { if (scheme->IsString()) {
Utf8Value value(env->isolate(), scheme); Utf8Value value(env->isolate(), scheme);
context->scheme.assign(*value, value.length()); context.scheme.assign(*value, value.length());
} }
Local<Value> port = GET(env, context_obj, "port"); Local<Value> port =
context_obj->Get(env->context(), env->port_string()).ToLocalChecked();
if (port->IsInt32()) if (port->IsInt32())
context->port = port->Int32Value(env->context()).FromJust(); context.port = port.As<Int32>()->Value();
if (context->flags & URL_FLAGS_HAS_USERNAME) { if (context.flags & URL_FLAGS_HAS_USERNAME) {
Local<Value> username = GET(env, context_obj, "username"); Local<Value> username =
context_obj->Get(env->context(),
env->username_string()).ToLocalChecked();
CHECK(username->IsString()); CHECK(username->IsString());
Utf8Value value(env->isolate(), username); Utf8Value value(env->isolate(), username);
context->username.assign(*value, value.length()); context.username.assign(*value, value.length());
} }
if (context->flags & URL_FLAGS_HAS_PASSWORD) { if (context.flags & URL_FLAGS_HAS_PASSWORD) {
Local<Value> password = GET(env, context_obj, "password"); Local<Value> password =
context_obj->Get(env->context(),
env->password_string()).ToLocalChecked();
CHECK(password->IsString()); CHECK(password->IsString());
Utf8Value value(env->isolate(), password); Utf8Value value(env->isolate(), password);
context->password.assign(*value, value.length()); context.password.assign(*value, value.length());
} }
Local<Value> host = GET(env, context_obj, "host"); Local<Value> host =
context_obj->Get(env->context(),
env->host_string()).ToLocalChecked();
if (host->IsString()) { if (host->IsString()) {
Utf8Value value(env->isolate(), host); Utf8Value value(env->isolate(), host);
context->host.assign(*value, value.length()); context.host.assign(*value, value.length());
} }
return context;
} }
// Single dot segment can be ".", "%2e", or "%2E" // Single dot segment can be ".", "%2e", or "%2E"
@ -1267,30 +1297,37 @@ void URL::Parse(const char* input,
len = end - p; len = end - p;
} }
// The spec says we should strip out any ASCII tabs or newlines.
// In those cases, we create another std::string instance with the filtered
// contents, but in the general case we avoid the overhead.
std::string whitespace_stripped; std::string whitespace_stripped;
whitespace_stripped.reserve(len); for (const char* ptr = p; ptr < end; ptr++) {
for (const char* ptr = p; ptr < end; ptr++) if (!IsASCIITabOrNewline(*ptr))
continue;
// Hit tab or newline. Allocate storage, copy what we have until now,
// and then iterate and filter all similar characters out.
whitespace_stripped.reserve(len - 1);
whitespace_stripped.assign(p, ptr - p);
// 'ptr + 1' skips the current char, which we know to be tab or newline.
for (ptr = ptr + 1; ptr < end; ptr++) {
if (!IsASCIITabOrNewline(*ptr)) if (!IsASCIITabOrNewline(*ptr))
whitespace_stripped += *ptr; whitespace_stripped += *ptr;
}
// Update variables like they should have looked like if the string
// had been stripped of whitespace to begin with.
input = whitespace_stripped.c_str(); input = whitespace_stripped.c_str();
len = whitespace_stripped.size(); len = whitespace_stripped.size();
p = input; p = input;
end = input + len; end = input + len;
break;
}
bool atflag = false; bool atflag = false; // Set when @ has been seen.
bool sbflag = false; bool square_bracket_flag = false; // Set inside of [...]
bool uflag = false; bool password_token_seen_flag = false; // Set after a : after an username.
std::string buffer; std::string buffer;
url->scheme.reserve(len);
url->username.reserve(len);
url->password.reserve(len);
url->host.reserve(len);
url->path.reserve(len);
url->query.reserve(len);
url->fragment.reserve(len);
buffer.reserve(len);
// Set the initial parse state. // Set the initial parse state.
const bool has_state_override = state_override != kUnknownState; const bool has_state_override = state_override != kUnknownState;
@ -1347,7 +1384,7 @@ void URL::Parse(const char* input,
// as it can be done before even entering C++ binding. // as it can be done before even entering C++ binding.
} }
url->scheme = buffer; url->scheme = std::move(buffer);
url->port = NormalizePort(url->scheme, url->port); url->port = NormalizePort(url->scheme, url->port);
if (new_is_special) { if (new_is_special) {
url->flags |= URL_FLAGS_SPECIAL; url->flags |= URL_FLAGS_SPECIAL;
@ -1373,7 +1410,7 @@ void URL::Parse(const char* input,
} else { } else {
url->flags |= URL_FLAGS_CANNOT_BE_BASE; url->flags |= URL_FLAGS_CANNOT_BE_BASE;
url->flags |= URL_FLAGS_HAS_PATH; url->flags |= URL_FLAGS_HAS_PATH;
url->path.push_back(""); url->path.emplace_back("");
state = kCannotBeBase; state = kCannotBeBase;
} }
} else if (!has_state_override) { } else if (!has_state_override) {
@ -1602,12 +1639,12 @@ void URL::Parse(const char* input,
const char bch = buffer[n]; const char bch = buffer[n];
if (bch == ':') { if (bch == ':') {
url->flags |= URL_FLAGS_HAS_PASSWORD; url->flags |= URL_FLAGS_HAS_PASSWORD;
if (!uflag) { if (!password_token_seen_flag) {
uflag = true; password_token_seen_flag = true;
continue; continue;
} }
} }
if (uflag) { if (password_token_seen_flag) {
AppendOrEscape(&url->password, bch, USERINFO_ENCODE_SET); AppendOrEscape(&url->password, bch, USERINFO_ENCODE_SET);
} else { } else {
AppendOrEscape(&url->username, bch, USERINFO_ENCODE_SET); AppendOrEscape(&url->username, bch, USERINFO_ENCODE_SET);
@ -1635,7 +1672,7 @@ void URL::Parse(const char* input,
if (has_state_override && url->scheme == "file:") { if (has_state_override && url->scheme == "file:") {
state = kFileHost; state = kFileHost;
continue; continue;
} else if (ch == ':' && !sbflag) { } else if (ch == ':' && !square_bracket_flag) {
if (buffer.size() == 0) { if (buffer.size() == 0) {
url->flags |= URL_FLAGS_FAILED; url->flags |= URL_FLAGS_FAILED;
return; return;
@ -1679,9 +1716,9 @@ void URL::Parse(const char* input,
} }
} else { } else {
if (ch == '[') if (ch == '[')
sbflag = true; square_bracket_flag = true;
if (ch == ']') if (ch == ']')
sbflag = false; square_bracket_flag = false;
buffer += ch; buffer += ch;
} }
break; break;
@ -1888,12 +1925,12 @@ void URL::Parse(const char* input,
ShortenUrlPath(url); ShortenUrlPath(url);
if (ch != '/' && !special_back_slash) { if (ch != '/' && !special_back_slash) {
url->flags |= URL_FLAGS_HAS_PATH; url->flags |= URL_FLAGS_HAS_PATH;
url->path.push_back(""); url->path.emplace_back("");
} }
} else if (IsSingleDotSegment(buffer) && } else if (IsSingleDotSegment(buffer) &&
ch != '/' && !special_back_slash) { ch != '/' && !special_back_slash) {
url->flags |= URL_FLAGS_HAS_PATH; url->flags |= URL_FLAGS_HAS_PATH;
url->path.push_back(""); url->path.emplace_back("");
} else if (!IsSingleDotSegment(buffer)) { } else if (!IsSingleDotSegment(buffer)) {
if (url->scheme == "file:" && if (url->scheme == "file:" &&
url->path.empty() && url->path.empty() &&
@ -1907,8 +1944,7 @@ void URL::Parse(const char* input,
buffer[1] = ':'; buffer[1] = ':';
} }
url->flags |= URL_FLAGS_HAS_PATH; url->flags |= URL_FLAGS_HAS_PATH;
std::string segment(buffer.c_str(), buffer.size()); url->path.emplace_back(std::move(buffer));
url->path.push_back(segment);
} }
buffer.clear(); buffer.clear();
if (url->scheme == "file:" && if (url->scheme == "file:" &&
@ -1947,7 +1983,7 @@ void URL::Parse(const char* input,
case kQuery: case kQuery:
if (ch == kEOL || (!has_state_override && ch == '#')) { if (ch == kEOL || (!has_state_override && ch == '#')) {
url->flags |= URL_FLAGS_HAS_QUERY; url->flags |= URL_FLAGS_HAS_QUERY;
url->query = buffer; url->query = std::move(buffer);
buffer.clear(); buffer.clear();
if (ch == '#') if (ch == '#')
state = kFragment; state = kFragment;
@ -1959,7 +1995,7 @@ void URL::Parse(const char* input,
switch (ch) { switch (ch) {
case kEOL: case kEOL:
url->flags |= URL_FLAGS_HAS_FRAGMENT; url->flags |= URL_FLAGS_HAS_FRAGMENT;
url->fragment = buffer; url->fragment = std::move(buffer);
break; break;
case 0: case 0:
break; break;
@ -1977,25 +2013,25 @@ void URL::Parse(const char* input,
} // NOLINT(readability/fn_size) } // NOLINT(readability/fn_size)
static inline void SetArgs(Environment* env, static inline void SetArgs(Environment* env,
Local<Value> argv[], Local<Value> argv[ARG_COUNT],
const struct url_data* url) { const struct url_data& url) {
Isolate* isolate = env->isolate(); Isolate* isolate = env->isolate();
argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url->flags); argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
argv[ARG_PROTOCOL] = OneByteString(isolate, url->scheme.c_str()); argv[ARG_PROTOCOL] = OneByteString(isolate, url.scheme.c_str());
if (url->flags & URL_FLAGS_HAS_USERNAME) if (url.flags & URL_FLAGS_HAS_USERNAME)
argv[ARG_USERNAME] = UTF8STRING(isolate, url->username); argv[ARG_USERNAME] = Utf8String(isolate, url.username);
if (url->flags & URL_FLAGS_HAS_PASSWORD) if (url.flags & URL_FLAGS_HAS_PASSWORD)
argv[ARG_PASSWORD] = UTF8STRING(isolate, url->password); argv[ARG_PASSWORD] = Utf8String(isolate, url.password);
if (url->flags & URL_FLAGS_HAS_HOST) if (url.flags & URL_FLAGS_HAS_HOST)
argv[ARG_HOST] = UTF8STRING(isolate, url->host); argv[ARG_HOST] = Utf8String(isolate, url.host);
if (url->flags & URL_FLAGS_HAS_QUERY) if (url.flags & URL_FLAGS_HAS_QUERY)
argv[ARG_QUERY] = UTF8STRING(isolate, url->query); argv[ARG_QUERY] = Utf8String(isolate, url.query);
if (url->flags & URL_FLAGS_HAS_FRAGMENT) if (url.flags & URL_FLAGS_HAS_FRAGMENT)
argv[ARG_FRAGMENT] = UTF8STRING(isolate, url->fragment); argv[ARG_FRAGMENT] = Utf8String(isolate, url.fragment);
if (url->port > -1) if (url.port > -1)
argv[ARG_PORT] = Integer::New(isolate, url->port); argv[ARG_PORT] = Integer::New(isolate, url.port);
if (url->flags & URL_FLAGS_HAS_PATH) if (url.flags & URL_FLAGS_HAS_PATH)
argv[ARG_PATH] = Copy(env, url->path); argv[ARG_PATH] = ToJSStringArray(env, url.path);
} }
static void Parse(Environment* env, static void Parse(Environment* env,
@ -2015,12 +2051,12 @@ static void Parse(Environment* env,
const bool has_context = context_obj->IsObject(); const bool has_context = context_obj->IsObject();
const bool has_base = base_obj->IsObject(); const bool has_base = base_obj->IsObject();
struct url_data base; url_data base;
struct url_data url; url_data url;
if (has_context) if (has_context)
HarvestContext(env, &url, context_obj.As<Object>()); url = HarvestContext(env, context_obj.As<Object>());
if (has_base) if (has_base)
HarvestBase(env, &base, base_obj.As<Object>()); base = HarvestBase(env, base_obj.As<Object>());
URL::Parse(input, len, state_override, &url, has_context, &base, has_base); URL::Parse(input, len, state_override, &url, has_context, &base, has_base);
if ((url.flags & URL_FLAGS_INVALID_PARSE_STATE) || if ((url.flags & URL_FLAGS_INVALID_PARSE_STATE) ||
@ -2032,7 +2068,7 @@ static void Parse(Environment* env,
const Local<Value> undef = Undefined(isolate); const Local<Value> undef = Undefined(isolate);
const Local<Value> null = Null(isolate); const Local<Value> null = Null(isolate);
if (!(url.flags & URL_FLAGS_FAILED)) { if (!(url.flags & URL_FLAGS_FAILED)) {
Local<Value> argv[9] = { Local<Value> argv[] = {
undef, undef,
undef, undef,
undef, undef,
@ -2043,7 +2079,7 @@ static void Parse(Environment* env,
null, // query defaults to null null, // query defaults to null
null, // fragment defaults to null null, // fragment defaults to null
}; };
SetArgs(env, argv, &url); SetArgs(env, argv, url);
cb->Call(context, recv, arraysize(argv), argv).FromMaybe(Local<Value>()); cb->Call(context, recv, arraysize(argv), argv).FromMaybe(Local<Value>());
} else if (error_cb->IsFunction()) { } else if (error_cb->IsFunction()) {
Local<Value> argv[2] = { undef, undef }; Local<Value> argv[2] = { undef, undef };
@ -2152,7 +2188,7 @@ static void DomainToASCII(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(FIXED_ONE_BYTE_STRING(env->isolate(), "")); args.GetReturnValue().Set(FIXED_ONE_BYTE_STRING(env->isolate(), ""));
return; return;
} }
std::string out = host.ToString(); std::string out = host.ToStringMove();
args.GetReturnValue().Set( args.GetReturnValue().Set(
String::NewFromUtf8(env->isolate(), String::NewFromUtf8(env->isolate(),
out.c_str(), out.c_str(),
@ -2172,7 +2208,7 @@ static void DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(FIXED_ONE_BYTE_STRING(env->isolate(), "")); args.GetReturnValue().Set(FIXED_ONE_BYTE_STRING(env->isolate(), ""));
return; return;
} }
std::string out = host.ToString(); std::string out = host.ToStringMove();
args.GetReturnValue().Set( args.GetReturnValue().Set(
String::NewFromUtf8(env->isolate(), String::NewFromUtf8(env->isolate(),
out.c_str(), out.c_str(),
@ -2255,7 +2291,7 @@ const Local<Value> URL::ToObject(Environment* env) const {
if (context_.flags & URL_FLAGS_FAILED) if (context_.flags & URL_FLAGS_FAILED)
return Local<Value>(); return Local<Value>();
Local<Value> argv[9] = { Local<Value> argv[] = {
undef, undef,
undef, undef,
undef, undef,
@ -2266,7 +2302,7 @@ const Local<Value> URL::ToObject(Environment* env) const {
null, // query defaults to null null, // query defaults to null
null, // fragment defaults to null null, // fragment defaults to null
}; };
SetArgs(env, argv, &context_); SetArgs(env, argv, context_);
MaybeLocal<Value> ret; MaybeLocal<Value> ret;
{ {