Skip to content

Commit 4c6bdff

Browse files
anonriganonrig
committed
url: simplify and improve url formatting
1 parent b4a962d commit 4c6bdff

File tree

4 files changed

+107
-102
lines changed

4 files changed

+107
-102
lines changed

‎lib/internal/url.js

Lines changed: 13 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ const path = require('path');
7777

7878
const {
7979
validateFunction,
80-
validateObject,
8180
} = require('internal/validators');
8281

8382
const querystring = require('querystring');
@@ -150,8 +149,6 @@ class URLContext {
150149
password = '';
151150
port = '';
152151
hash = '';
153-
hasHost = false;
154-
hasOpaquePath = false;
155152
}
156153

157154
function isURLSearchParams(self) {
@@ -282,7 +279,9 @@ class URLSearchParams {
282279
name = toUSVString(name);
283280
value = toUSVString(value);
284281
ArrayPrototypePush(this[searchParams], name, value);
285-
update(this[context], this);
282+
if (this[context]) {
283+
this[context].search = this.toString();
284+
}
286285
}
287286

288287
delete(name) {
@@ -303,7 +302,9 @@ class URLSearchParams {
303302
i += 2;
304303
}
305304
}
306-
update(this[context], this);
305+
if (this[context]) {
306+
this[context].search = this.toString();
307+
}
307308
}
308309

309310
get(name) {
@@ -398,7 +399,9 @@ class URLSearchParams {
398399
ArrayPrototypePush(list, name, value);
399400
}
400401

401-
update(this[context], this);
402+
if (this[context]) {
403+
this[context].search = this.toString();
404+
}
402405
}
403406

404407
sort() {
@@ -442,7 +445,9 @@ class URLSearchParams {
442445
}
443446
}
444447

445-
update(this[context], this);
448+
if (this[context]) {
449+
this[context].search = this.toString();
450+
}
446451
}
447452

448453
// https://heycam.github.io/webidl/#es-iterators
@@ -528,46 +533,6 @@ function isURLThis(self) {
528533
return (self !== undefined && self !== null && self[context] !== undefined);
529534
}
530535

531-
function constructHref(ctx, options) {
532-
if (options)
533-
validateObject(options, 'options');
534-
535-
options = {
536-
fragment: true,
537-
unicode: false,
538-
search: true,
539-
auth: true,
540-
...options
541-
};
542-
543-
// https://url.spec.whatwg.org/#url-serializing
544-
let ret = ctx.protocol;
545-
if (ctx.hasHost) {
546-
ret += '//';
547-
const hasUsername = ctx.username !== '';
548-
const hasPassword = ctx.password !== '';
549-
if (options.auth && (hasUsername || hasPassword)) {
550-
if (hasUsername)
551-
ret += ctx.username;
552-
if (hasPassword)
553-
ret += `:${ctx.password}`;
554-
ret += '@';
555-
}
556-
ret += options.unicode ?
557-
domainToUnicode(ctx.hostname) : ctx.hostname;
558-
if (ctx.port !== '')
559-
ret += `:${ctx.port}`;
560-
} else if (!ctx.hasOpaquePath && ctx.pathname.lastIndexOf('/') !== 0 && ctx.pathname.startsWith('//')) {
561-
ret += '/.';
562-
}
563-
ret += ctx.pathname;
564-
if (options.search)
565-
ret += ctx.search;
566-
if (options.fragment)
567-
ret += ctx.hash;
568-
return ret;
569-
}
570-
571536
class URL {
572537
constructor(input, base = undefined) {
573538
// toUSVString is not needed.
@@ -620,14 +585,8 @@ class URL {
620585
return `${constructor.name} ${inspect(obj, opts)}`;
621586
}
622587

623-
[kFormat](options) {
624-
// TODO(@anonrig): Replace kFormat with actually calling setters.
625-
return constructHref(this[context], options);
626-
}
627-
628588
#onParseComplete = (href, origin, protocol, hostname, pathname,
629-
search, username, password, port, hash, hasHost,
630-
hasOpaquePath) => {
589+
search, username, password, port, hash) => {
631590
const ctx = this[context];
632591
ctx.href = href;
633592
ctx.origin = origin;
@@ -639,9 +598,6 @@ class URL {
639598
ctx.password = password;
640599
ctx.port = port;
641600
ctx.hash = hash;
642-
// TODO(@anonrig): Remove hasHost and hasOpaquePath when kFormat is removed.
643-
ctx.hasHost = hasHost;
644-
ctx.hasOpaquePath = hasOpaquePath;
645601
if (!this[searchParams]) { // Invoked from URL constructor
646602
this[searchParams] = new URLSearchParams();
647603
this[searchParams][context] = this;
@@ -854,33 +810,6 @@ ObjectDefineProperties(URL, {
854810
revokeObjectURL: kEnumerableProperty,
855811
});
856812

857-
function update(url, params) {
858-
if (!url)
859-
return;
860-
861-
const ctx = url[context];
862-
const serializedParams = params.toString();
863-
if (serializedParams.length > 0) {
864-
ctx.search = '?' + serializedParams;
865-
} else {
866-
ctx.search = '';
867-
868-
// Potentially strip trailing spaces from an opaque path
869-
if (ctx.hasOpaquePath && ctx.hash.length === 0) {
870-
let length = ctx.pathname.length;
871-
while (length > 0 && ctx.pathname.charCodeAt(length - 1) === 32) {
872-
length--;
873-
}
874-
875-
// No need to copy the whole string if there is no space at the end
876-
if (length !== ctx.pathname.length) {
877-
ctx.pathname = ctx.pathname.slice(0, length);
878-
}
879-
}
880-
}
881-
ctx.href = constructHref(ctx);
882-
}
883-
884813
function initSearchParams(url, init) {
885814
if (!init) {
886815
url[searchParams] = [];
@@ -1379,7 +1308,6 @@ module.exports = {
13791308
domainToASCII,
13801309
domainToUnicode,
13811310
urlToHttpOptions,
1382-
formatSymbol: kFormat,
13831311
searchParamsSymbol: searchParams,
13841312
encodeStr
13851313
};

‎lib/url.js

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
'use strict';
2323

2424
const {
25+
Boolean,
2526
Int8Array,
2627
ObjectKeys,
2728
SafeSet,
@@ -37,7 +38,10 @@ const {
3738
ERR_INVALID_ARG_TYPE,
3839
ERR_INVALID_URL,
3940
} = require('internal/errors').codes;
40-
const { validateString } = require('internal/validators');
41+
const {
42+
validateString,
43+
validateObject,
44+
} = require('internal/validators');
4145

4246
// This ensures setURLConstructor() is called before the native
4347
// URL::ToObject() method is used.
@@ -50,11 +54,14 @@ const {
5054
domainToASCII,
5155
domainToUnicode,
5256
fileURLToPath,
53-
formatSymbol,
5457
pathToFileURL,
5558
urlToHttpOptions,
5659
} = require('internal/url');
5760

61+
const {
62+
formatUrl,
63+
} = internalBinding('url');
64+
5865
// Original url.parse() API
5966

6067
function Url() {
@@ -587,11 +594,35 @@ function urlFormat(urlObject, options) {
587594
} else if (typeof urlObject !== 'object' || urlObject === null) {
588595
throw new ERR_INVALID_ARG_TYPE('urlObject',
589596
['Object', 'string'], urlObject);
597+
} else if (urlObject instanceof URL) {
598+
let fragment = true;
599+
let unicode = false;
600+
let search = true;
601+
let auth = true;
602+
603+
if (options) {
604+
validateObject(options, 'options');
605+
606+
if (options.fragment != null) {
607+
fragment = Boolean(options.fragment);
608+
}
609+
610+
if (options.unicode != null) {
611+
unicode = Boolean(options.unicode);
612+
}
613+
614+
if (options.search != null) {
615+
search = Boolean(options.search);
616+
}
617+
618+
if (options.auth != null) {
619+
auth = Boolean(options.auth);
620+
}
621+
}
622+
623+
return formatUrl(urlObject.href, fragment, unicode, search, auth);
590624
} else if (!(urlObject instanceof Url)) {
591-
const format = urlObject[formatSymbol];
592-
return format ?
593-
format.call(urlObject, options) :
594-
Url.prototype.format.call(urlObject);
625+
return Url.prototype.format.call(urlObject);
595626
}
596627
return urlObject.format();
597628
}

‎src/node_url.cc

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
namespace node {
1313

14-
using v8::Boolean;
1514
using v8::Context;
1615
using v8::Function;
1716
using v8::FunctionCallbackInfo;
@@ -47,7 +46,7 @@ enum url_update_action {
4746
kHref = 9,
4847
};
4948

50-
void SetArgs(Environment* env, Local<Value> argv[12], const ada::result& url) {
49+
void SetArgs(Environment* env, Local<Value> argv[10], const ada::result& url) {
5150
Isolate* isolate = env->isolate();
5251
argv[0] = Utf8String(isolate, url->get_href());
5352
argv[1] = Utf8String(isolate, url->get_origin());
@@ -59,8 +58,6 @@ void SetArgs(Environment* env, Local<Value> argv[12], const ada::result& url) {
5958
argv[7] = Utf8String(isolate, url->get_password());
6059
argv[8] = Utf8String(isolate, url->get_port());
6160
argv[9] = Utf8String(isolate, url->get_hash());
62-
argv[10] = Boolean::New(isolate, url->host.has_value());
63-
argv[11] = Boolean::New(isolate, url->has_opaque_path);
6461
}
6562

6663
void Parse(const FunctionCallbackInfo<Value>& args) {
@@ -105,8 +102,6 @@ void Parse(const FunctionCallbackInfo<Value>& args) {
105102
undef,
106103
undef,
107104
undef,
108-
undef,
109-
undef,
110105
};
111106
SetArgs(env, argv, out);
112107
USE(success_callback_->Call(
@@ -255,21 +250,73 @@ void UpdateUrl(const FunctionCallbackInfo<Value>& args) {
255250
undef,
256251
undef,
257252
undef,
258-
undef,
259-
undef,
260253
};
261254
SetArgs(env, argv, out);
262255
USE(success_callback_->Call(
263256
env->context(), args.This(), arraysize(argv), argv));
264257
args.GetReturnValue().Set(result);
265258
}
266259

260+
void FormatUrl(const FunctionCallbackInfo<Value>& args) {
261+
CHECK_GT(args.Length(), 4);
262+
CHECK(args[0]->IsString()); // url href
263+
264+
Environment* env = Environment::GetCurrent(args);
265+
Isolate* isolate = env->isolate();
266+
267+
Utf8Value href(isolate, args[0].As<String>());
268+
const bool fragment = args[1]->IsTrue();
269+
const bool unicode = args[2]->IsTrue();
270+
const bool search = args[3]->IsTrue();
271+
const bool auth = args[4]->IsTrue();
272+
273+
ada::result out = ada::parse(std::string_view(href.out(), href.length()));
274+
CHECK(out);
275+
276+
if (!fragment) {
277+
out->set_hash("");
278+
}
279+
280+
if (unicode) {
281+
#if defined(NODE_HAVE_I18N_SUPPORT)
282+
std::string hostname = out->get_hostname();
283+
MaybeStackBuffer<char> buf;
284+
int32_t len = i18n::ToUnicode(&buf, hostname.data(), hostname.length());
285+
286+
if (len < 0) {
287+
out->host = "";
288+
} else {
289+
out->host = std::string(buf.out(), len);
290+
}
291+
#else
292+
out->host = "";
293+
#endif
294+
}
295+
296+
if (!search) {
297+
out->set_search("");
298+
}
299+
300+
if (!auth) {
301+
out->set_username("");
302+
out->set_password("");
303+
}
304+
305+
std::string result = out->get_href();
306+
args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
307+
result.data(),
308+
NewStringType::kNormal,
309+
result.length())
310+
.ToLocalChecked());
311+
}
312+
267313
void Initialize(Local<Object> target,
268314
Local<Value> unused,
269315
Local<Context> context,
270316
void* priv) {
271317
SetMethod(context, target, "parse", Parse);
272318
SetMethod(context, target, "updateUrl", UpdateUrl);
319+
SetMethod(context, target, "formatUrl", FormatUrl);
273320

274321
SetMethodNoSideEffect(context, target, "domainToASCII", DomainToASCII);
275322
SetMethodNoSideEffect(context, target, "domainToUnicode", DomainToUnicode);
@@ -279,6 +326,7 @@ void Initialize(Local<Object> target,
279326
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
280327
registry->Register(Parse);
281328
registry->Register(UpdateUrl);
329+
registry->Register(FormatUrl);
282330

283331
registry->Register(DomainToASCII);
284332
registry->Register(DomainToUnicode);

‎test/parallel/test-whatwg-url-custom-inspect.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,7 @@ assert.strictEqual(
5555
username: 'username',
5656
password: 'password',
5757
port: '8080',
58-
hash: '#hash',
59-
hasHost: true,
60-
hasOpaquePath: false
58+
hash: '#hash'
6159
}
6260
}`);
6361

0 commit comments

Comments
 (0)