Changeset 295509 in webkit for trunk/Source/JavaScriptCore/jsc.cpp

Timestamp:

Jun 13, 2022, 6:00:05 PM (3 years ago)

Author:

Ross Kirsling

Message:

jsc.exe --module-file should understand Windows paths
​https://bugs.webkit.org/show_bug.cgi?id=241518

Reviewed by Yusuke Suzuki.

jsc.cpp's module loader was written without any accommodation for Windows, so:

1. On Windows, recognize C:\foo as an absolute path and .\foo and ..\foo as dotted relative paths (allowing '/' too).
2. On all platforms, stop misusing the URL(base, relative) constructor. This isn't the way to add ​file:/// to an abspath.

This ensures that module tests are able to run well on Windows.

• Source/JavaScriptCore/jsc.cpp:

(isAbsolutePath): Added.
(isDottedRelativePath): Added.
(absoluteFileURL): Renamed from absolutePath.
(GlobalObject::moduleLoaderImportModule):
(GlobalObject::moduleLoaderResolve):
(JSC_DEFINE_HOST_FUNCTION):
(computeFilePath):
(runWithOptions):

Canonical link: ​https://commits.webkit.org/251514@main

File:

• trunk/Source/JavaScriptCore/jsc.cpp (modified) (9 diffs)

trunk/Source/JavaScriptCore/jsc.cpp

@@ -845 +845 @@
 }
 
-static URL absolutePath(const String& fileName)
-{
+// FIXME: We may wish to support module specifiers beginning with a (back)slash on Windows. We could either:
+// - align with V8 and SM:  treat '/foo' as './foo'
+// - align with PowerShell: treat '/foo' as 'C:/foo'
+static bool isAbsolutePath(StringView path)
+{
+#if OS(WINDOWS)
+    // Just look for local drives like C:\.
+    return path.length() > 2 && isASCIIAlpha(path[0]) && path[1] == ':' && (path[2] == '\\' || path[2] == '/');
+#else
+    return path.startsWith('/');
+#endif
+}
+
+static bool isDottedRelativePath(StringView path)
+{
+#if OS(WINDOWS)
+    auto length = path.length();
+    if (length < 2 || path[0] != '.')
+        return false;
+
+    if (path[1] == '/' || path[1] == '\\')
+        return true;
+
+    return length > 2 && path[1] == '.' && (path[2] == '/' || path[2] == '\\');
+#else
+    return path.startsWith("./"_s) || path.startsWith("../"_s);
+#endif
+}
+
+static URL absoluteFileURL(const String& fileName)
+{
+    if (isAbsolutePath(fileName))
+        return URL::fileURLWithFileSystemPath(fileName);
+
     auto directoryName = currentWorkingDirectory();
     if (!directoryName.isValid())
@@ -873 +905 @@
         RELEASE_AND_RETURN(scope, rejectWithError(createError(globalObject, makeString("Could not resolve the referrer's path '", referrer.string(), "', while trying to resolve module '", specifier, "'."))));
 
-    if (!specifier.startsWith('/') && !specifier.startsWith("./"_s) && !specifier.startsWith("../"_s))
-        RELEASE_AND_RETURN(scope, rejectWithError(createTypeError(globalObject, makeString("Module specifier, '"_s, specifier, "' does not start with \"/\", \"./\", or \"../\". Referenced from: "_s, referrer.fileSystemPath()))));
-
-    URL moduleURL(referrer, specifier);
+    bool specifierIsAbsolute = isAbsolutePath(specifier);
+    if (!specifierIsAbsolute && !isDottedRelativePath(specifier))
+        RELEASE_AND_RETURN(scope, rejectWithError(createTypeError(globalObject, makeString("Module specifier, '"_s, specifier, "' is not absolute and does not start with \"./\" or \"../\". Referenced from: "_s, referrer.fileSystemPath()))));
+
+    auto moduleURL = specifierIsAbsolute ? URL::fileURLWithFileSystemPath(specifier) : URL(referrer, specifier);
     if (!moduleURL.isLocalFile())
         RELEASE_AND_RETURN(scope, rejectWithError(createError(globalObject, makeString("Module url, '", moduleURL.string(), "' does not map to a local file."))));
@@ -900 +933 @@
     auto resolvePath = [&] (const URL& directoryURL) -> Identifier {
         String specifier = key.impl();
-        if (!specifier.startsWith('/') && !specifier.startsWith("./"_s) && !specifier.startsWith("../"_s)) {
-            throwTypeError(globalObject, scope, makeString("Module specifier, '"_s, specifier, "' does not start with \"/\", \"./\", or \"../\". Referenced from: "_s, directoryURL.fileSystemPath()));
+        bool specifierIsAbsolute = isAbsolutePath(specifier);
+        if (!specifierIsAbsolute && !isDottedRelativePath(specifier)) {
+            throwTypeError(globalObject, scope, makeString("Module specifier, '"_s, specifier, "' is not absolute and does not start with \"./\" or \"../\". Referenced from: "_s, directoryURL.fileSystemPath()));
             return { };
         }
@@ -910 +944 @@
         }
 
-        URL resolvedURL(directoryURL, specifier);
+        auto resolvedURL = specifierIsAbsolute ? URL::fileURLWithFileSystemPath(specifier) : URL(directoryURL, specifier);
         if (!resolvedURL.isValid()) {
             throwException(globalObject, scope, createError(globalObject, makeString("Resolved module url is not valid: ", resolvedURL.string())));
@@ -1551 +1585 @@
     StopWatch stopWatch;
     stopWatch.start();
-    evaluate(realm, jscSource(script, SourceOrigin { absolutePath(fileName) }, fileName), JSValue(), exception);
+    evaluate(realm, jscSource(script, SourceOrigin { absoluteFileURL(fileName) }, fileName), JSValue(), exception);
     stopWatch.stop();
 
@@ -1613 +1647 @@
         }
     } else
-        path = absolutePath(fileName);
+        path = absoluteFileURL(fileName);
     return path;
 }
@@ -1703 +1737 @@
 
     JSValue syntaxException;
-    bool validSyntax = checkSyntax(globalObject, jscSource(script, SourceOrigin { absolutePath(fileName) }, fileName), &syntaxException);
+    bool validSyntax = checkSyntax(globalObject, jscSource(script, SourceOrigin { absoluteFileURL(fileName) }, fileName), &syntaxException);
     stopWatch.stop();
 
@@ -3252 +3286 @@
 
             if (isModule) {
-                // If the passed file isn't an absolute path append "./" so the module loader doesn't think this is a bare-name specifier.
-                fileName = fileName.startsWith('/') ? fileName : makeString("./", fileName);
+                // If necessary, prepend "./" so the module loader doesn't think this is a bare-name specifier.
+                fileName = isAbsolutePath(fileName) || isDottedRelativePath(fileName) ? fileName : makeString('.', pathSeparator(), fileName);
                 promise = loadAndEvaluateModule(globalObject, fileName, jsUndefined(), jsUndefined());
                 RETURN_IF_EXCEPTION(scope, void());
@@ -3270 +3304 @@
 
         bool isLastFile = i == scripts.size() - 1;
-        SourceOrigin sourceOrigin { absolutePath(fileName) };
+        SourceOrigin sourceOrigin { absoluteFileURL(fileName) };
         if (isModule) {
             if (!promise) {