Skip to content

Commit a39dd37

Browse files
anonrigdanielleadams
anonrig
authored and
danielleadams
committed
url: simplify and improve url formatting
PR-URL: #46736 Backport-PR-URL: #47435 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent e4cd81b commit a39dd37

File tree

4 files changed

+111
-110
lines changed

4 files changed

+111
-110
lines changed

‎lib/internal/url.js‎

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

8080
const{
8181
validateFunction,
82-
validateObject,
8382
}=require('internal/validators');
8483

8584
constquerystring=require('querystring');
@@ -152,8 +151,6 @@ class URLContext{
152151
password='';
153152
port='';
154153
hash='';
155-
hasHost=false;
156-
hasOpaquePath=false;
157154
}
158155

159156
functionisURLSearchParams(self){
@@ -290,7 +287,9 @@ class URLSearchParams{
290287
name=toUSVString(name);
291288
value=toUSVString(value);
292289
ArrayPrototypePush(this[searchParams],name,value);
293-
update(this[context],this);
290+
if(this[context]){
291+
this[context].search=this.toString();
292+
}
294293
}
295294

296295
delete(name){
@@ -311,7 +310,9 @@ class URLSearchParams{
311310
i+=2;
312311
}
313312
}
314-
update(this[context],this);
313+
if(this[context]){
314+
this[context].search=this.toString();
315+
}
315316
}
316317

317318
get(name){
@@ -406,7 +407,9 @@ class URLSearchParams{
406407
ArrayPrototypePush(list,name,value);
407408
}
408409

409-
update(this[context],this);
410+
if(this[context]){
411+
this[context].search=this.toString();
412+
}
410413
}
411414

412415
sort(){
@@ -450,7 +453,9 @@ class URLSearchParams{
450453
}
451454
}
452455

453-
update(this[context],this);
456+
if(this[context]){
457+
this[context].search=this.toString();
458+
}
454459
}
455460

456461
// https://heycam.github.io/webidl/#es-iterators
@@ -536,46 +541,6 @@ function isURLThis(self){
536541
returnself!=null&&ObjectPrototypeHasOwnProperty(self,context);
537542
}
538543

539-
functionconstructHref(ctx,options){
540-
if(options)
541-
validateObject(options,'options');
542-
543-
options={
544-
fragment: true,
545-
unicode: false,
546-
search: true,
547-
auth: true,
548-
...options,
549-
};
550-
551-
// https://url.spec.whatwg.org/#url-serializing
552-
letret=ctx.protocol;
553-
if(ctx.hasHost){
554-
ret+='//';
555-
consthasUsername=ctx.username!=='';
556-
consthasPassword=ctx.password!=='';
557-
if(options.auth&&(hasUsername||hasPassword)){
558-
if(hasUsername)
559-
ret+=ctx.username;
560-
if(hasPassword)
561-
ret+=`:${ctx.password}`;
562-
ret+='@';
563-
}
564-
ret+=options.unicode ?
565-
domainToUnicode(ctx.hostname) : ctx.hostname;
566-
if(ctx.port!=='')
567-
ret+=`:${ctx.port}`;
568-
}elseif(!ctx.hasOpaquePath&&ctx.pathname.lastIndexOf('/')!==0&&ctx.pathname.startsWith('//')){
569-
ret+='/.';
570-
}
571-
ret+=ctx.pathname;
572-
if(options.search)
573-
ret+=ctx.search;
574-
if(options.fragment)
575-
ret+=ctx.hash;
576-
returnret;
577-
}
578-
579544
classURL{
580545
constructor(input,base=undefined){
581546
// toUSVString is not needed.
@@ -628,14 +593,8 @@ class URL{
628593
return`${constructor.name}${inspect(obj,opts)}`;
629594
}
630595

631-
[kFormat](options){
632-
// TODO(@anonrig): Replace kFormat with actually calling setters.
633-
returnconstructHref(this[context],options);
634-
}
635-
636596
#onParseComplete =(href,origin,protocol,hostname,pathname,
637-
search,username,password,port,hash,hasHost,
638-
hasOpaquePath)=>{
597+
search,username,password,port,hash)=>{
639598
constctx=this[context];
640599
ctx.href=href;
641600
ctx.origin=origin;
@@ -647,9 +606,6 @@ class URL{
647606
ctx.password=password;
648607
ctx.port=port;
649608
ctx.hash=hash;
650-
// TODO(@anonrig): Remove hasHost and hasOpaquePath when kFormat is removed.
651-
ctx.hasHost=hasHost;
652-
ctx.hasOpaquePath=hasOpaquePath;
653609
if(!this[searchParams]){// Invoked from URL constructor
654610
this[searchParams]=newURLSearchParams();
655611
this[searchParams][context]=this;
@@ -862,33 +818,6 @@ ObjectDefineProperties(URL,{
862818
revokeObjectURL: kEnumerableProperty,
863819
});
864820

865-
functionupdate(url,params){
866-
if(!url)
867-
return;
868-
869-
constctx=url[context];
870-
constserializedParams=params.toString();
871-
if(serializedParams.length>0){
872-
ctx.search='?'+serializedParams;
873-
}else{
874-
ctx.search='';
875-
876-
// Potentially strip trailing spaces from an opaque path
877-
if(ctx.hasOpaquePath&&ctx.hash.length===0){
878-
letlength=ctx.pathname.length;
879-
while(length>0&&ctx.pathname.charCodeAt(length-1)===32){
880-
length--;
881-
}
882-
883-
// No need to copy the whole string if there is no space at the end
884-
if(length!==ctx.pathname.length){
885-
ctx.pathname=ctx.pathname.slice(0,length);
886-
}
887-
}
888-
}
889-
ctx.href=constructHref(ctx);
890-
}
891-
892821
functioninitSearchParams(url,init){
893822
if(!init){
894823
url[searchParams]=[];
@@ -1387,7 +1316,6 @@ module.exports ={
13871316
domainToASCII,
13881317
domainToUnicode,
13891318
urlToHttpOptions,
1390-
formatSymbol: kFormat,
13911319
searchParamsSymbol: searchParams,
13921320
encodeStr,
13931321
};

‎lib/url.js‎

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

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

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

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

6168
functionUrl(){
@@ -579,13 +586,36 @@ function urlFormat(urlObject, options){
579586
}elseif(typeofurlObject!=='object'||urlObject===null){
580587
thrownewERR_INVALID_ARG_TYPE('urlObject',
581588
['Object','string'],urlObject);
582-
}elseif(!(urlObjectinstanceofUrl)){
583-
constformat=urlObject[formatSymbol];
584-
returnformat ?
585-
format.call(urlObject,options) :
586-
Url.prototype.format.call(urlObject);
589+
}elseif(urlObjectinstanceofURL){
590+
letfragment=true;
591+
letunicode=false;
592+
letsearch=true;
593+
letauth=true;
594+
595+
if(options){
596+
validateObject(options,'options');
597+
598+
if(options.fragment!=null){
599+
fragment=Boolean(options.fragment);
600+
}
601+
602+
if(options.unicode!=null){
603+
unicode=Boolean(options.unicode);
604+
}
605+
606+
if(options.search!=null){
607+
search=Boolean(options.search);
608+
}
609+
610+
if(options.auth!=null){
611+
auth=Boolean(options.auth);
612+
}
613+
}
614+
615+
returnformatUrl(urlObject.href,fragment,unicode,search,auth);
587616
}
588-
returnurlObject.format();
617+
618+
returnUrl.prototype.format.call(urlObject);
589619
}
590620

591621
// These characters do not need escaping:

‎src/node_url.cc‎

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

1212
namespacenode{
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-
voidSetArgs(Environment* env, Local<Value> argv[12], const ada::result& url){
49+
voidSetArgs(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
voidParse(const FunctionCallbackInfo<Value>& args){
@@ -86,8 +83,7 @@ void Parse(const FunctionCallbackInfo<Value>& args){
8683
}
8784
base_pointer = &base.value();
8885
}
89-
ada::result out =
90-
ada::parse(std::string_view(input.out(), input.length()), base_pointer);
86+
ada::result out = ada::parse(input.ToStringView(), base_pointer);
9187

9288
if (!out){
9389
return args.GetReturnValue().Set(false);
@@ -105,8 +101,6 @@ void Parse(const FunctionCallbackInfo<Value>& args){
105101
undef,
106102
undef,
107103
undef,
108-
undef,
109-
undef,
110104
};
111105
SetArgs(env, argv, out);
112106
USE(success_callback_->Call(
@@ -192,10 +186,8 @@ void UpdateUrl(const FunctionCallbackInfo<Value>& args){
192186
Utf8Value new_value(isolate, args[2].As<String>());
193187
Local<Function> success_callback_ = args[3].As<Function>();
194188

195-
std::string_view new_value_view =
196-
std::string_view(new_value.out(), new_value.length());
197-
std::string_view input_view = std::string_view(input.out(), input.length());
198-
ada::result out = ada::parse(input_view);
189+
std::string_view new_value_view = new_value.ToStringView();
190+
ada::result out = ada::parse(input.ToStringView());
199191
CHECK(out);
200192

201193
bool result{true};
@@ -255,21 +247,73 @@ void UpdateUrl(const FunctionCallbackInfo<Value>& args){
255247
undef,
256248
undef,
257249
undef,
258-
undef,
259-
undef,
260250
};
261251
SetArgs(env, argv, out);
262252
USE(success_callback_->Call(
263253
env->context(), args.This(), arraysize(argv), argv));
264254
args.GetReturnValue().Set(result);
265255
}
266256

257+
voidFormatUrl(const FunctionCallbackInfo<Value>& args){
258+
CHECK_GT(args.Length(), 4);
259+
CHECK(args[0]->IsString()); // url href
260+
261+
Environment* env = Environment::GetCurrent(args);
262+
Isolate* isolate = env->isolate();
263+
264+
Utf8Value href(isolate, args[0].As<String>());
265+
constbool fragment = args[1]->IsTrue();
266+
constbool unicode = args[2]->IsTrue();
267+
constbool search = args[3]->IsTrue();
268+
constbool auth = args[4]->IsTrue();
269+
270+
ada::result out = ada::parse(href.ToStringView());
271+
CHECK(out);
272+
273+
if (!fragment){
274+
out->fragment = std::nullopt;
275+
}
276+
277+
if (unicode){
278+
#if defined(NODE_HAVE_I18N_SUPPORT)
279+
std::string hostname = out->get_hostname();
280+
MaybeStackBuffer<char> buf;
281+
int32_t len = i18n::ToUnicode(&buf, hostname.data(), hostname.length());
282+
283+
if (len < 0){
284+
out->host = "";
285+
} else{
286+
out->host = buf.ToString();
287+
}
288+
#else
289+
out->host = "";
290+
#endif
291+
}
292+
293+
if (!search){
294+
out->query = std::nullopt;
295+
}
296+
297+
if (!auth){
298+
out->username = "";
299+
out->password = "";
300+
}
301+
302+
std::string result = out->get_href();
303+
args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
304+
result.data(),
305+
NewStringType::kNormal,
306+
result.length())
307+
.ToLocalChecked());
308+
}
309+
267310
voidInitialize(Local<Object> target,
268311
Local<Value> unused,
269312
Local<Context> context,
270313
void* priv){
271314
SetMethod(context, target, "parse", Parse);
272315
SetMethod(context, target, "updateUrl", UpdateUrl);
316+
SetMethod(context, target, "formatUrl", FormatUrl);
273317

274318
SetMethodNoSideEffect(context, target, "domainToASCII", DomainToASCII);
275319
SetMethodNoSideEffect(context, target, "domainToUnicode", DomainToUnicode);
@@ -279,6 +323,7 @@ void Initialize(Local<Object> target,
279323
voidRegisterExternalReferences(ExternalReferenceRegistry* registry){
280324
registry->Register(Parse);
281325
registry->Register(UpdateUrl);
326+
registry->Register(FormatUrl);
282327

283328
registry->Register(DomainToASCII);
284329
registry->Register(DomainToUnicode);

0 commit comments

Comments
(0)