[webkit-changes] [295509] trunk/Source/JavaScriptCore/jsc.cpp

Mon, 13 Jun 2022 18:00:48 -0700

Title: [295509] trunk/Source/_javascript_Core/jsc.cpp
Revision
295509
Author
ross.kirsl...@sony.com
Date
2022-06-13 18:00:05 -0700 (Mon, 13 Jun 2022)

Log 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/_javascript_Core/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

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/jsc.cpp (295508 => 295509)


--- trunk/Source/_javascript_Core/jsc.cpp	2022-06-14 00:41:13 UTC (rev 295508)
+++ trunk/Source/_javascript_Core/jsc.cpp	2022-06-14 01:00:05 UTC (rev 295509)
@@ -844,8 +844,40 @@
     return URL::fileURLWithFileSystemPath(directoryString);
 }
 
-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())
         return URL::fileURLWithFileSystemPath(fileName);
@@ -872,10 +904,11 @@
     if (!referrer.isLocalFile())
         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()))));
+    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()))));
 
-    URL moduleURL(referrer, specifier);
+    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."))));
 
@@ -899,8 +932,9 @@
 
     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 { };
         }
 
@@ -909,7 +943,7 @@
             return { };
         }
 
-        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())));
             return { };
@@ -1550,7 +1584,7 @@
     NakedPtr<Exception> exception;
     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();
 
     if (exception) {
@@ -1612,7 +1646,7 @@
             return URL();
         }
     } else
-        path = absolutePath(fileName);
+        path = absoluteFileURL(fileName);
     return path;
 }
 
@@ -1702,7 +1736,7 @@
     stopWatch.start();
 
     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();
 
     if (!validSyntax)
@@ -3251,8 +3285,8 @@
                 scriptBuffer.append("\"use strict\";\n", strlen("\"use strict\";\n"));
 
             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());
             } else {
@@ -3269,7 +3303,7 @@
         }
 
         bool isLastFile = i == scripts.size() - 1;
-        SourceOrigin sourceOrigin { absolutePath(fileName) };
+        SourceOrigin sourceOrigin { absoluteFileURL(fileName) };
         if (isModule) {
             if (!promise) {
                 // FIXME: This should use an absolute file URL https://bugs.webkit.org/show_bug.cgi?id=193077

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to