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

url: origin of "blob:" URL containing inner non-"http(s):" URL #48168

Closed
wants to merge 2 commits into from
Closed
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
3 changes: 3 additions & 0 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,9 @@ class URL {
if (path.length > 0) {
try {
const out = new URL(path);
if (out.#context.scheme_type === 0 || out.#context.scheme_type === 2) {
return 'null';
}
Copy link
Member

@anonrig anonrig May 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The standard says that

If pathURL’s scheme is not "http" and not "https", then return a new opaque origin.

For performance reasons, we avoid having string comparisons and calling getters which runs a string.prototype.slice. We can however use scheme_type numeric value.

If you look into the declaration of scheme_type variable in lib/internal/url.js, you can find the following comment:

  /**
   * Refers to `ada::scheme::type`
   *
   * enum type : uint8_t {
   *   HTTP = 0,
   *   NOT_SPECIAL = 1,
   *   HTTPS = 2,
   *   WS = 3,
   *   FTP = 4,
   *   WSS = 5,
   *   FILE = 6
   * };
   * @type {number}
   */

For this particular case: we need to change line 846 to:

if (out.#context.scheme_type === 0 || out.#context.scheme_type === 2)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thank you. I did notice out.#context.scheme_type !== 1 but didn't pay attention to the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anonrig commit looks better, now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this enum be exported to js in some way? would be good to not just have magic numbers floating around

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the very least, please add a comment indicating where the numbers come from

if (out.#context.scheme_type !== 1) {
return `${out.protocol}//${out.host}`;
}
Expand Down
56 changes: 28 additions & 28 deletions lib/internal/v8/startup_snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,100 +2,100 @@

const {
validateFunction,
} = require('internal/validators');
}=require('internal/validators');
const {
codes: {
ERR_NOT_BUILDING_SNAPSHOT,
ERR_NOT_SUPPORTED_IN_SNAPSHOT,
ERR_DUPLICATE_STARTUP_SNAPSHOT_MAIN_FUNCTION,
},
} = require('internal/errors');
}=require('internal/errors');

const {
setSerializeCallback,
setDeserializeCallback,
setDeserializeMainFunction: _setDeserializeMainFunction,
isBuildingSnapshotBuffer,
} = internalBinding('mksnapshot');
}=internalBinding('mksnapshot');

function isBuildingSnapshot() {
return isBuildingSnapshotBuffer[0];
}

function throwIfNotBuildingSnapshot() {
if (!isBuildingSnapshot()) {
if(!isBuildingSnapshot()) {
throw new ERR_NOT_BUILDING_SNAPSHOT();
}
}

function throwIfBuildingSnapshot(reason) {
if (isBuildingSnapshot()) {
if(isBuildingSnapshot()) {
throw new ERR_NOT_SUPPORTED_IN_SNAPSHOT(reason);
}
}

const deserializeCallbacks = [];
let deserializeCallbackIsSet = false;
const deserializeCallbacks=[];
let deserializeCallbackIsSet=false;
function runDeserializeCallbacks() {
while (deserializeCallbacks.length > 0) {
const { 0: callback, 1: data } = deserializeCallbacks.shift();
while(deserializeCallbacks.length>0) {
const {0: callback,1: data}=deserializeCallbacks.shift();
callback(data);
}
}

function addDeserializeCallback(callback, data) {
function addDeserializeCallback(callback,data) {
throwIfNotBuildingSnapshot();
validateFunction(callback, 'callback');
if (!deserializeCallbackIsSet) {
validateFunction(callback,'callback');
if(!deserializeCallbackIsSet) {
// TODO(joyeecheung): when the main function handling is done in JS,
// the deserialize callbacks can always be invoked. For now only
// store it in C++ when it's actually used to avoid unnecessary
// C++ -> JS costs.
setDeserializeCallback(runDeserializeCallbacks);
deserializeCallbackIsSet = true;
deserializeCallbackIsSet=true;
}
deserializeCallbacks.push([callback, data]);
deserializeCallbacks.push([callback,data]);
}

const serializeCallbacks = [];
const serializeCallbacks=[];
function runSerializeCallbacks() {
while (serializeCallbacks.length > 0) {
const { 0: callback, 1: data } = serializeCallbacks.shift();
while(serializeCallbacks.length>0) {
const {0: callback,1: data}=serializeCallbacks.shift();
callback(data);
}
}

function addSerializeCallback(callback, data) {
function addSerializeCallback(callback,data) {
throwIfNotBuildingSnapshot();
validateFunction(callback, 'callback');
serializeCallbacks.push([callback, data]);
validateFunction(callback,'callback');
serializeCallbacks.push([callback,data]);
}

function initializeCallbacks() {
// Only run the serialize callbacks in snapshot building mode, otherwise
// they throw.
if (isBuildingSnapshot()) {
if(isBuildingSnapshot()) {
setSerializeCallback(runSerializeCallbacks);
}
}

let deserializeMainIsSet = false;
function setDeserializeMainFunction(callback, data) {
let deserializeMainIsSet=false;
function setDeserializeMainFunction(callback,data) {
throwIfNotBuildingSnapshot();
// TODO(joyeecheung): In lib/internal/bootstrap/node.js, create a default
// main function to run the lib/internal/main scripts and make sure that
// the main function set in the snapshot building process takes precedence.
validateFunction(callback, 'callback');
if (deserializeMainIsSet) {
validateFunction(callback,'callback');
if(deserializeMainIsSet) {
throw new ERR_DUPLICATE_STARTUP_SNAPSHOT_MAIN_FUNCTION();
}
deserializeMainIsSet = true;
deserializeMainIsSet=true;

_setDeserializeMainFunction(function deserializeMain() {
const {
prepareMainThreadExecution,
markBootstrapComplete,
} = require('internal/process/pre_execution');
}=require('internal/process/pre_execution');

// This should be in sync with run_main_module.js until we make that
// a built-in main function.
Expand All @@ -106,7 +106,7 @@ function setDeserializeMainFunction(callback, data) {
});
}

module.exports = {
module.exports={
initializeCallbacks,
runDeserializeCallbacks,
throwIfBuildingSnapshot,
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/wpt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Last update:
- resource-timing: https://github.com/web-platform-tests/wpt/tree/22d38586d0/resource-timing
- resources: https://github.com/web-platform-tests/wpt/tree/919874f84f/resources
- streams: https://github.com/web-platform-tests/wpt/tree/51750bc8d7/streams
- url: https://github.com/web-platform-tests/wpt/tree/c4726447f3/url
- url: https://github.com/web-platform-tests/wpt/tree/84782d9315/url
- user-timing: https://github.com/web-platform-tests/wpt/tree/df24fb604e/user-timing
- wasm/jsapi: https://github.com/web-platform-tests/wpt/tree/cde25e7e3c/wasm/jsapi
- wasm/webapi: https://github.com/web-platform-tests/wpt/tree/fd1b23eeaa/wasm/webapi
Expand Down
8 changes: 0 additions & 8 deletions test/fixtures/wpt/url/resources/percent-encoding.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,5 @@
"output": {
"utf-8": "%C3%A1|"
}
},
"Surrogate!",
{
"input": "\ud800",
"output": {
"utf-8": "%EF%BF%BD",
"windows-1252": "%26%2365533%3B"
}
}
]
168 changes: 168 additions & 0 deletions test/fixtures/wpt/url/resources/urltestdata.json
Original file line number Diff line number Diff line change
Expand Up @@ -7755,6 +7755,21 @@
"search": "",
"hash": ""
},
{
"input": "blob:http://example.org:88/",
"base": null,
"href": "blob:http://example.org:88/",
"origin": "http://example.org:88",
"protocol": "blob:",
"username": "",
"password": "",
"host": "",
"hostname": "",
"port": "",
"pathname": "http://example.org:88/",
"search": "",
"hash": ""
},
{
"input": "blob:d3958f5c-0777-0845-9dcf-2cb28783acaf",
"base": null,
Expand Down Expand Up @@ -7785,6 +7800,129 @@
"search": "",
"hash": ""
},
"blob: in blob:",
{
"input": "blob:blob:",
"base": null,
"href": "blob:blob:",
"origin": "null",
"protocol": "blob:",
"username": "",
"password": "",
"host": "",
"hostname": "",
"port": "",
"pathname": "blob:",
"search": "",
"hash": ""
},
{
"input": "blob:blob:https://example.org/",
"base": null,
"href": "blob:blob:https://example.org/",
"origin": "null",
"protocol": "blob:",
"username": "",
"password": "",
"host": "",
"hostname": "",
"port": "",
"pathname": "blob:https://example.org/",
"search": "",
"hash": ""
},
"Non-http(s): in blob:",
{
"input": "blob:about:blank",
"base": null,
"href": "blob:about:blank",
"origin": "null",
"protocol": "blob:",
"username": "",
"password": "",
"host": "",
"hostname": "",
"port": "",
"pathname": "about:blank",
"search": "",
"hash": ""
},
{
"input": "blob:file://host/path",
"base": null,
"href": "blob:file://host/path",
"origin": "null",
"protocol": "blob:",
"username": "",
"password": "",
"host": "",
"hostname": "",
"port": "",
"pathname": "file://host/path",
"search": "",
"hash": ""
},
{
"input": "blob:ftp://host/path",
"base": null,
"href": "blob:ftp://host/path",
"origin": "null",
"protocol": "blob:",
"username": "",
"password": "",
"host": "",
"hostname": "",
"port": "",
"pathname": "ftp://host/path",
"search": "",
"hash": ""
},
{
"input": "blob:ws://example.org/",
"base": null,
"href": "blob:ws://example.org/",
"origin": "null",
"protocol": "blob:",
"username": "",
"password": "",
"host": "",
"hostname": "",
"port": "",
"pathname": "ws://example.org/",
"search": "",
"hash": ""
},
{
"input": "blob:wss://example.org/",
"base": null,
"href": "blob:wss://example.org/",
"origin": "null",
"protocol": "blob:",
"username": "",
"password": "",
"host": "",
"hostname": "",
"port": "",
"pathname": "wss://example.org/",
"search": "",
"hash": ""
},
"Percent-encoded http: in blob:",
{
"input": "blob:http%3a//example.org/",
"base": null,
"href": "blob:http%3a//example.org/",
"origin": "null",
"protocol": "blob:",
"username": "",
"password": "",
"host": "",
"hostname": "",
"port": "",
"pathname": "http%3a//example.org/",
"search": "",
"hash": ""
},
"Invalid IPv4 radix digits",
{
"input": "http://0x7f.0.0.0x7g",
Expand Down Expand Up @@ -9362,5 +9500,35 @@
"input": "stun://[:1]",
"base": null,
"failure": true
},
{
"input": "w://x:0",
"base": null,
"href": "w://x:0",
"origin": "null",
"protocol": "w:",
"username": "",
"password": "",
"host": "x:0",
"hostname": "x",
"port": "0",
"pathname": "",
"search": "",
"hash": ""
},
{
"input": "west://x:0",
"base": null,
"href": "west://x:0",
"origin": "null",
"protocol": "west:",
"username": "",
"password": "",
"host": "x:0",
"hostname": "x",
"port": "0",
"pathname": "",
"search": "",
"hash": ""
}
]
2 changes: 1 addition & 1 deletion test/fixtures/wpt/versions.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
"path": "streams"
},
"url": {
"commit": "c4726447f3bad1c71244fb99c98738ff0354a034",
"commit": "84782d931516aa13cfe32dc7eaa1377b4d947d66",
"path": "url"
},
"user-timing": {
Expand Down