Skip to content

Commit

Permalink
url: file URL path normalization
Browse files Browse the repository at this point in the history
Refs: whatwg/url#544
Refs: web-platform-tests/wpt#25716

PR-URL: #35477
Fixes: #35429
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
watilde committed Oct 12, 2020
1 parent a844387 commit bb62f4a
Show file tree
Hide file tree
Showing 8 changed files with 214 additions and 101 deletions.
2 changes: 1 addition & 1 deletion lib/internal/bootstrap/pre_execution.js
Expand Up @@ -362,7 +362,7 @@ function initializePolicy() {
// no bare specifiers for now
let manifestURL;
if (require('path').isAbsolute(experimentalPolicy)) {
manifestURL = new URL(`file:///${experimentalPolicy}`);
manifestURL = new URL(experimentalPolicy, 'file://');
} else {
const cwdURL = pathToFileURL(process.cwd());
cwdURL.pathname += '/';
Expand Down
2 changes: 1 addition & 1 deletion src/inspector_agent.cc
Expand Up @@ -640,7 +640,7 @@ class NodeInspectorClient : public V8InspectorClient {
node::url::URL url = node::url::URL::FromFilePath(resource_name);
// TODO(ak239spb): replace this code with url.href().
// Refs: https://github.com/nodejs/node/issues/22610
return Utf8ToStringView(url.protocol() + "//" + url.path());
return Utf8ToStringView(url.protocol() + "/" + url.path());
}

node::Environment* env_;
Expand Down
52 changes: 19 additions & 33 deletions src/node_url.cc
Expand Up @@ -1904,16 +1904,19 @@ void URL::Parse(const char* input,
state = kFragment;
break;
default:
url->query.clear();
if (base->flags & URL_FLAGS_HAS_HOST) {
url->flags |= URL_FLAGS_HAS_HOST;
url->host = base->host;
}
if (base->flags & URL_FLAGS_HAS_PATH) {
url->flags |= URL_FLAGS_HAS_PATH;
url->path = base->path;
}
if (!StartsWithWindowsDriveLetter(p, end)) {
if (base->flags & URL_FLAGS_HAS_HOST) {
url->flags |= URL_FLAGS_HAS_HOST;
url->host = base->host;
}
if (base->flags & URL_FLAGS_HAS_PATH) {
url->flags |= URL_FLAGS_HAS_PATH;
url->path = base->path;
}
ShortenUrlPath(url);
} else {
url->path.clear();
}
state = kPath;
continue;
Expand All @@ -1927,20 +1930,13 @@ void URL::Parse(const char* input,
if (ch == '/' || ch == '\\') {
state = kFileHost;
} else {
if (has_base &&
base->scheme == "file:" &&
!StartsWithWindowsDriveLetter(p, end)) {
if (IsNormalizedWindowsDriveLetter(base->path[0])) {
if (has_base && base->scheme == "file:") {
url->flags |= URL_FLAGS_HAS_HOST;
url->host = base->host;
if (!StartsWithWindowsDriveLetter(p, end) &&
IsNormalizedWindowsDriveLetter(base->path[0])) {
url->flags |= URL_FLAGS_HAS_PATH;
url->path.push_back(base->path[0]);
} else {
if (base->flags & URL_FLAGS_HAS_HOST) {
url->flags |= URL_FLAGS_HAS_HOST;
url->host = base->host;
} else {
url->flags &= ~URL_FLAGS_HAS_HOST;
url->host.clear();
}
}
}
state = kPath;
Expand Down Expand Up @@ -2024,29 +2020,19 @@ void URL::Parse(const char* input,
url->path.empty() &&
buffer.size() == 2 &&
IsWindowsDriveLetter(buffer)) {
if ((url->flags & URL_FLAGS_HAS_HOST) &&
!url->host.empty()) {
url->host.clear();
url->flags |= URL_FLAGS_HAS_HOST;
}
buffer[1] = ':';
}
url->flags |= URL_FLAGS_HAS_PATH;
url->path.emplace_back(std::move(buffer));
}
buffer.clear();
if (url->scheme == "file:" &&
(ch == kEOL ||
ch == '?' ||
ch == '#')) {
while (url->path.size() > 1 && url->path[0].empty()) {
url->path.erase(url->path.begin());
}
}
if (ch == '?') {
url->flags |= URL_FLAGS_HAS_QUERY;
url->query.clear();
state = kQuery;
} else if (ch == '#') {
url->flags |= URL_FLAGS_HAS_FRAGMENT;
url->fragment.clear();
state = kFragment;
}
} else {
Expand Down
6 changes: 3 additions & 3 deletions test/cctest/test_url.cc
Expand Up @@ -148,14 +148,14 @@ TEST_F(URLTest, FromFilePath) {
#else
file_url = URL::FromFilePath("/");
EXPECT_EQ("file:", file_url.protocol());
EXPECT_EQ("/", file_url.path());
EXPECT_EQ("//", file_url.path());

file_url = URL::FromFilePath("/a/b/c");
EXPECT_EQ("file:", file_url.protocol());
EXPECT_EQ("/a/b/c", file_url.path());
EXPECT_EQ("//a/b/c", file_url.path());

file_url = URL::FromFilePath("/a/%%.js");
EXPECT_EQ("file:", file_url.protocol());
EXPECT_EQ("/a/%25%25.js", file_url.path());
EXPECT_EQ("//a/%25%25.js", file_url.path());
#endif
}
2 changes: 1 addition & 1 deletion test/fixtures/wpt/README.md
Expand Up @@ -12,7 +12,7 @@ Last update:

- console: https://github.com/web-platform-tests/wpt/tree/3b1f72e99a/console
- encoding: https://github.com/web-platform-tests/wpt/tree/d7f9e16c9a/encoding
- url: https://github.com/web-platform-tests/wpt/tree/e2ddf48b78/url
- url: https://github.com/web-platform-tests/wpt/tree/050308a616/url
- resources: https://github.com/web-platform-tests/wpt/tree/1d14e821b9/resources
- interfaces: https://github.com/web-platform-tests/wpt/tree/15e47f779c/interfaces
- html/webappapis/microtask-queuing: https://github.com/web-platform-tests/wpt/tree/2c5c3c4c27/html/webappapis/microtask-queuing
Expand Down
12 changes: 6 additions & 6 deletions test/fixtures/wpt/url/resources/setters_tests.json
Expand Up @@ -1672,26 +1672,26 @@
"href": "file://monkey/",
"new_value": "\\\\",
"expected": {
"href": "file://monkey/",
"pathname": "/"
"href": "file://monkey//",
"pathname": "//"
}
},
{
"comment": "File URLs and (back)slashes",
"href": "file:///unicorn",
"new_value": "//\\/",
"expected": {
"href": "file:///",
"pathname": "/"
"href": "file://////",
"pathname": "////"
}
},
{
"comment": "File URLs and (back)slashes",
"href": "file:///unicorn",
"new_value": "//monkey/..//",
"expected": {
"href": "file:///",
"pathname": "/"
"href": "file://///",
"pathname": "///"
}
},
{
Expand Down

0 comments on commit bb62f4a

Please sign in to comment.