Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: reduce URL invocations on http2 origins #48338

Merged
merged 2 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const EventEmitter = require('events');
const fs = require('fs');
const http = require('http');
const { readUInt16BE, readUInt32BE } = require('internal/buffer');
const { URL } = require('internal/url');
const { URL, getURLOrigin } = require('internal/url');
const net = require('net');
const { Duplex } = require('stream');
const tls = require('tls');
Expand Down Expand Up @@ -636,7 +636,7 @@ function initOriginSet(session) {
// We have to ensure that it is a properly serialized
// ASCII origin string. The socket.servername might not
// be properly ASCII encoded.
originSet.add((new URL(originString)).origin);
originSet.add(getURLOrigin(originString));
}
}
return originSet;
Expand Down Expand Up @@ -1641,7 +1641,7 @@ class ServerHttp2Session extends Http2Session {
let origin;

if (typeof originOrStream === 'string') {
origin = (new URL(originOrStream)).origin;
origin = getURLOrigin(originOrStream);
if (origin === 'null')
throw new ERR_HTTP2_ALTSVC_INVALID_ORIGIN();
} else if (typeof originOrStream === 'number') {
Expand Down Expand Up @@ -1693,7 +1693,7 @@ class ServerHttp2Session extends Http2Session {
for (let i = 0; i < count; i++) {
let origin = origins[i];
if (typeof origin === 'string') {
origin = (new URL(origin)).origin;
origin = getURLOrigin(origin);
} else if (origin != null && typeof origin === 'object') {
origin = origin.origin;
}
Expand Down
11 changes: 11 additions & 0 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,16 @@ function toPathIfFileURL(fileURLOrPath) {
return fileURLToPath(fileURLOrPath);
}

/**
* This util takes a string containing a URL and return the URL origin,
* its meant to avoid calls to `new URL` constructor.
* @param {string} url
* @returns {URL['origin']}
*/
function getURLOrigin(url) {
anonrig marked this conversation as resolved.
Show resolved Hide resolved
return bindingUrl.getOrigin(url);
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
}

module.exports = {
toUSVString,
fileURLToPath,
Expand All @@ -1451,4 +1461,5 @@ module.exports = {
isURL,

urlUpdateActions: updateActions,
getURLOrigin,
};
1 change: 1 addition & 0 deletions src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ void AppendExceptionLine(Environment* env,
V(ERR_INVALID_STATE, Error) \
V(ERR_INVALID_THIS, TypeError) \
V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \
V(ERR_INVALID_URL, TypeError) \
V(ERR_MEMORY_ALLOCATION_FAILED, Error) \
V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE, Error) \
V(ERR_MISSING_ARGS, TypeError) \
Expand Down
26 changes: 26 additions & 0 deletions src/node_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,30 @@ void BindingData::DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
.ToLocalChecked());
}

void BindingData::GetOrigin(const v8::FunctionCallbackInfo<Value>& args) {
CHECK_GE(args.Length(), 1);
CHECK(args[0]->IsString()); // input

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

Utf8Value input(env->isolate(), args[0]);
std::string_view input_view = input.ToStringView();
auto out = ada::parse<ada::url_aggregator>(input_view);

if (!out) {
THROW_ERR_INVALID_URL(env, "Invalid URL");
return;
}

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

void BindingData::CanParse(const FunctionCallbackInfo<Value>& args) {
CHECK_GE(args.Length(), 1);
CHECK(args[0]->IsString()); // input
Expand Down Expand Up @@ -322,6 +346,7 @@ void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
SetMethodNoSideEffect(isolate, target, "domainToASCII", DomainToASCII);
SetMethodNoSideEffect(isolate, target, "domainToUnicode", DomainToUnicode);
SetMethodNoSideEffect(isolate, target, "format", Format);
SetMethodNoSideEffect(isolate, target, "getOrigin", GetOrigin);
SetMethod(isolate, target, "parse", Parse);
SetMethod(isolate, target, "update", Update);
SetFastMethodNoSideEffect(
Expand All @@ -341,6 +366,7 @@ void BindingData::RegisterExternalReferences(
registry->Register(DomainToASCII);
registry->Register(DomainToUnicode);
registry->Register(Format);
registry->Register(GetOrigin);
registry->Register(Parse);
registry->Register(Update);
registry->Register(CanParse);
Expand Down
1 change: 1 addition & 0 deletions src/node_url.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class BindingData : public SnapshotableObject {
const v8::FastOneByteString& input);

static void Format(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetOrigin(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Parse(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Update(const v8::FunctionCallbackInfo<v8::Value>& args);

Expand Down