Skip to content

Commit

Permalink
url: simplify and improve url formatting
Browse files Browse the repository at this point in the history
PR-URL: #46736
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
anonrig committed Apr 11, 2023
1 parent 76fde06 commit aae4141
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 110 deletions.
98 changes: 13 additions & 85 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ const path = require('path');

const {
validateFunction,
validateObject,
} = require('internal/validators');

const querystring = require('querystring');
Expand Down Expand Up @@ -152,8 +151,6 @@ class URLContext {
password = '';
port = '';
hash = '';
hasHost = false;
hasOpaquePath = false;
}

function isURLSearchParams(self) {
Expand Down Expand Up @@ -290,7 +287,9 @@ class URLSearchParams {
name = toUSVString(name);
value = toUSVString(value);
ArrayPrototypePush(this[searchParams], name, value);
update(this[context], this);
if (this[context]) {
this[context].search = this.toString();
}
}

delete(name) {
Expand All @@ -311,7 +310,9 @@ class URLSearchParams {
i += 2;
}
}
update(this[context], this);
if (this[context]) {
this[context].search = this.toString();
}
}

get(name) {
Expand Down Expand Up @@ -406,7 +407,9 @@ class URLSearchParams {
ArrayPrototypePush(list, name, value);
}

update(this[context], this);
if (this[context]) {
this[context].search = this.toString();
}
}

sort() {
Expand Down Expand Up @@ -450,7 +453,9 @@ class URLSearchParams {
}
}

update(this[context], this);
if (this[context]) {
this[context].search = this.toString();
}
}

// https://heycam.github.io/webidl/#es-iterators
Expand Down Expand Up @@ -536,46 +541,6 @@ function isURLThis(self) {
return self != null && ObjectPrototypeHasOwnProperty(self, context);
}

function constructHref(ctx, options) {
if (options)
validateObject(options, 'options');

options = {
fragment: true,
unicode: false,
search: true,
auth: true,
...options,
};

// https://url.spec.whatwg.org/#url-serializing
let ret = ctx.protocol;
if (ctx.hasHost) {
ret += '//';
const hasUsername = ctx.username !== '';
const hasPassword = ctx.password !== '';
if (options.auth && (hasUsername || hasPassword)) {
if (hasUsername)
ret += ctx.username;
if (hasPassword)
ret += `:${ctx.password}`;
ret += '@';
}
ret += options.unicode ?
domainToUnicode(ctx.hostname) : ctx.hostname;
if (ctx.port !== '')
ret += `:${ctx.port}`;
} else if (!ctx.hasOpaquePath && ctx.pathname.lastIndexOf('/') !== 0 && ctx.pathname.startsWith('//')) {
ret += '/.';
}
ret += ctx.pathname;
if (options.search)
ret += ctx.search;
if (options.fragment)
ret += ctx.hash;
return ret;
}

class URL {
constructor(input, base = undefined) {
// toUSVString is not needed.
Expand Down Expand Up @@ -628,14 +593,8 @@ class URL {
return `${constructor.name} ${inspect(obj, opts)}`;
}

[kFormat](options) {
// TODO(@anonrig): Replace kFormat with actually calling setters.
return constructHref(this[context], options);
}

#onParseComplete = (href, origin, protocol, hostname, pathname,
search, username, password, port, hash, hasHost,
hasOpaquePath) => {
search, username, password, port, hash) => {
const ctx = this[context];
ctx.href = href;
ctx.origin = origin;
Expand All @@ -647,9 +606,6 @@ class URL {
ctx.password = password;
ctx.port = port;
ctx.hash = hash;
// TODO(@anonrig): Remove hasHost and hasOpaquePath when kFormat is removed.
ctx.hasHost = hasHost;
ctx.hasOpaquePath = hasOpaquePath;
if (!this[searchParams]) { // Invoked from URL constructor
this[searchParams] = new URLSearchParams();
this[searchParams][context] = this;
Expand Down Expand Up @@ -862,33 +818,6 @@ ObjectDefineProperties(URL, {
revokeObjectURL: kEnumerableProperty,
});

function update(url, params) {
if (!url)
return;

const ctx = url[context];
const serializedParams = params.toString();
if (serializedParams.length > 0) {
ctx.search = '?' + serializedParams;
} else {
ctx.search = '';

// Potentially strip trailing spaces from an opaque path
if (ctx.hasOpaquePath && ctx.hash.length === 0) {
let length = ctx.pathname.length;
while (length > 0 && ctx.pathname.charCodeAt(length - 1) === 32) {
length--;
}

// No need to copy the whole string if there is no space at the end
if (length !== ctx.pathname.length) {
ctx.pathname = ctx.pathname.slice(0, length);
}
}
}
ctx.href = constructHref(ctx);
}

function initSearchParams(url, init) {
if (!init) {
url[searchParams] = [];
Expand Down Expand Up @@ -1387,7 +1316,6 @@ module.exports = {
domainToASCII,
domainToUnicode,
urlToHttpOptions,
formatSymbol: kFormat,
searchParamsSymbol: searchParams,
encodeStr,
};
46 changes: 38 additions & 8 deletions lib/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
'use strict';

const {
Boolean,
Int8Array,
ObjectCreate,
ObjectKeys,
Expand All @@ -38,7 +39,10 @@ const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_URL,
} = require('internal/errors').codes;
const { validateString } = require('internal/validators');
const {
validateString,
validateObject,
} = require('internal/validators');

// This ensures setURLConstructor() is called before the native
// URL::ToObject() method is used.
Expand All @@ -51,11 +55,14 @@ const {
domainToASCII,
domainToUnicode,
fileURLToPath,
formatSymbol,
pathToFileURL,
urlToHttpOptions,
} = require('internal/url');

const {
formatUrl,
} = internalBinding('url');

// Original url.parse() API

function Url() {
Expand Down Expand Up @@ -579,13 +586,36 @@ function urlFormat(urlObject, options) {
} else if (typeof urlObject !== 'object' || urlObject === null) {
throw new ERR_INVALID_ARG_TYPE('urlObject',
['Object', 'string'], urlObject);
} else if (!(urlObject instanceof Url)) {
const format = urlObject[formatSymbol];
return format ?
format.call(urlObject, options) :
Url.prototype.format.call(urlObject);
} else if (urlObject instanceof URL) {
let fragment = true;
let unicode = false;
let search = true;
let auth = true;

if (options) {
validateObject(options, 'options');

if (options.fragment != null) {
fragment = Boolean(options.fragment);
}

if (options.unicode != null) {
unicode = Boolean(options.unicode);
}

if (options.search != null) {
search = Boolean(options.search);
}

if (options.auth != null) {
auth = Boolean(options.auth);
}
}

return formatUrl(urlObject.href, fragment, unicode, search, auth);
}
return urlObject.format();

return Url.prototype.format.call(urlObject);
}

// These characters do not need escaping:
Expand Down
73 changes: 59 additions & 14 deletions src/node_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace node {

using v8::Boolean;
using v8::Context;
using v8::Function;
using v8::FunctionCallbackInfo;
Expand Down Expand Up @@ -47,7 +46,7 @@ enum url_update_action {
kHref = 9,
};

void SetArgs(Environment* env, Local<Value> argv[12], const ada::result& url) {
void SetArgs(Environment* env, Local<Value> argv[10], const ada::result& url) {
Isolate* isolate = env->isolate();
argv[0] = Utf8String(isolate, url->get_href());
argv[1] = Utf8String(isolate, url->get_origin());
Expand All @@ -59,8 +58,6 @@ void SetArgs(Environment* env, Local<Value> argv[12], const ada::result& url) {
argv[7] = Utf8String(isolate, url->get_password());
argv[8] = Utf8String(isolate, url->get_port());
argv[9] = Utf8String(isolate, url->get_hash());
argv[10] = Boolean::New(isolate, url->host.has_value());
argv[11] = Boolean::New(isolate, url->has_opaque_path);
}

void Parse(const FunctionCallbackInfo<Value>& args) {
Expand All @@ -86,8 +83,7 @@ void Parse(const FunctionCallbackInfo<Value>& args) {
}
base_pointer = &base.value();
}
ada::result out =
ada::parse(std::string_view(input.out(), input.length()), base_pointer);
ada::result out = ada::parse(input.ToStringView(), base_pointer);

if (!out) {
return args.GetReturnValue().Set(false);
Expand All @@ -105,8 +101,6 @@ void Parse(const FunctionCallbackInfo<Value>& args) {
undef,
undef,
undef,
undef,
undef,
};
SetArgs(env, argv, out);
USE(success_callback_->Call(
Expand Down Expand Up @@ -192,10 +186,8 @@ void UpdateUrl(const FunctionCallbackInfo<Value>& args) {
Utf8Value new_value(isolate, args[2].As<String>());
Local<Function> success_callback_ = args[3].As<Function>();

std::string_view new_value_view =
std::string_view(new_value.out(), new_value.length());
std::string_view input_view = std::string_view(input.out(), input.length());
ada::result out = ada::parse(input_view);
std::string_view new_value_view = new_value.ToStringView();
ada::result out = ada::parse(input.ToStringView());
CHECK(out);

bool result{true};
Expand Down Expand Up @@ -255,21 +247,73 @@ void UpdateUrl(const FunctionCallbackInfo<Value>& args) {
undef,
undef,
undef,
undef,
undef,
};
SetArgs(env, argv, out);
USE(success_callback_->Call(
env->context(), args.This(), arraysize(argv), argv));
args.GetReturnValue().Set(result);
}

void FormatUrl(const FunctionCallbackInfo<Value>& args) {
CHECK_GT(args.Length(), 4);
CHECK(args[0]->IsString()); // url href

Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();

Utf8Value href(isolate, args[0].As<String>());
const bool fragment = args[1]->IsTrue();
const bool unicode = args[2]->IsTrue();
const bool search = args[3]->IsTrue();
const bool auth = args[4]->IsTrue();

ada::result out = ada::parse(href.ToStringView());
CHECK(out);

if (!fragment) {
out->fragment = std::nullopt;
}

if (unicode) {
#if defined(NODE_HAVE_I18N_SUPPORT)
std::string hostname = out->get_hostname();
MaybeStackBuffer<char> buf;
int32_t len = i18n::ToUnicode(&buf, hostname.data(), hostname.length());

if (len < 0) {
out->host = "";
} else {
out->host = buf.ToString();
}
#else
out->host = "";
#endif
}

if (!search) {
out->query = std::nullopt;
}

if (!auth) {
out->username = "";
out->password = "";
}

std::string result = out->get_href();
args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
result.data(),
NewStringType::kNormal,
result.length())
.ToLocalChecked());
}

void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
SetMethod(context, target, "parse", Parse);
SetMethod(context, target, "updateUrl", UpdateUrl);
SetMethod(context, target, "formatUrl", FormatUrl);

SetMethodNoSideEffect(context, target, "domainToASCII", DomainToASCII);
SetMethodNoSideEffect(context, target, "domainToUnicode", DomainToUnicode);
Expand All @@ -279,6 +323,7 @@ void Initialize(Local<Object> target,
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(Parse);
registry->Register(UpdateUrl);
registry->Register(FormatUrl);

registry->Register(DomainToASCII);
registry->Register(DomainToUnicode);
Expand Down
Loading

0 comments on commit aae4141

Please sign in to comment.