Skip to content

Commit 16f2f34

Browse files
hnshns
committed
Add base URI argument throughout module loading code path.
This is necessary for relative module IDs (starting with "." or "..") which are resolved relative to the loading module to keep track of the base repository in sandboxed environments. Also enhances test for relative require to trigger this bug.
1 parent 2442919 commit 16f2f34

File tree

12 files changed

+35
-25
lines changed

12 files changed

+35
-25
lines changed

‎src/org/mozilla/javascript/commonjs/module/ModuleScriptProvider.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ public interface ModuleScriptProvider
2424
* @param moduleUri the URI of the module. If this is not null, resolution
2525
* of <code>moduleId</code> is bypassed and the script is directly loaded
2626
* from <code>moduleUri</code>
27+
* @param baseUri the module path base URI from which <code>moduleUri</code>
28+
* was derived.
2729
* @param paths the value of the require() function's "paths" attribute. If
2830
* the require() function is sandboxed, it will be null, otherwise it will
2931
* be a JavaScript Array object. It is up to the provider implementation
@@ -36,7 +38,7 @@ public interface ModuleScriptProvider
3638
* valid absolute module identifier.
3739
*/
3840
public ModuleScript getModuleScript(Context cx, String moduleId,
39-
URI moduleUri, Scriptable paths)
41+
URI moduleUri, URI baseUri, Scriptable paths)
4042
throws Exception;
4143

4244
}

‎src/org/mozilla/javascript/commonjs/module/Require.java‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public Scriptable requireMain(Context cx, String mainModuleId) {
122122
try {
123123
// try to get the module script to see if it is on the module path
124124
moduleScript = moduleScriptProvider.getModuleScript(
125-
cx, mainModuleId, null, paths);
125+
cx, mainModuleId, null, null, paths);
126126
} catch (RuntimeException x) {
127127
throw x;
128128
} catch (Exception x) {
@@ -131,7 +131,7 @@ public Scriptable requireMain(Context cx, String mainModuleId) {
131131

132132
if (moduleScript != null) {
133133
mainExports = getExportedModuleInterface(cx, mainModuleId,
134-
null, true);
134+
null, null, true);
135135
} else if (!sandboxed) {
136136

137137
URI mainUri = null;
@@ -153,7 +153,7 @@ public Scriptable requireMain(Context cx, String mainModuleId) {
153153
mainUri = file.toURI();
154154
}
155155
mainExports = getExportedModuleInterface(cx, mainUri.toString(),
156-
mainUri, true);
156+
mainUri, null, true);
157157
}
158158

159159
this.mainModuleId = mainModuleId;
@@ -179,6 +179,7 @@ public Object call(Context cx, Scriptable scope, Scriptable thisObj,
179179

180180
String id = (String)Context.jsToJava(args[0], String.class);
181181
URI uri = null;
182+
URI base = null;
182183
if (id.startsWith("./") || id.startsWith("../")) {
183184
if (!(thisObj instanceof ModuleScope)) {
184185
throw ScriptRuntime.throwError(cx, scope,
@@ -187,7 +188,7 @@ public Object call(Context cx, Scriptable scope, Scriptable thisObj,
187188
}
188189

189190
ModuleScope moduleScope = (ModuleScope) thisObj;
190-
URI base = moduleScope.getBase();
191+
base = moduleScope.getBase();
191192
URI current = moduleScope.getUri();
192193
uri = current.resolve(id);
193194

@@ -210,7 +211,7 @@ public Object call(Context cx, Scriptable scope, Scriptable thisObj,
210211
}
211212
}
212213
}
213-
return getExportedModuleInterface(cx, id, uri, false);
214+
return getExportedModuleInterface(cx, id, uri, base, false);
214215
}
215216

216217
public Scriptable construct(Context cx, Scriptable scope, Object[] args) {
@@ -219,7 +220,7 @@ public Scriptable construct(Context cx, Scriptable scope, Object[] args) {
219220
}
220221

221222
private Scriptable getExportedModuleInterface(Context cx, String id,
222-
URI uri, boolean isMain)
223+
URI uri, URI base, boolean isMain)
223224
{
224225
// Check if the requested module is already completely loaded
225226
Scriptable exports = exportedModuleInterfaces.get(id);
@@ -256,7 +257,7 @@ private Scriptable getExportedModuleInterface(Context cx, String id,
256257
return exports;
257258
}
258259
// Nope, still not loaded; we're loading it then.
259-
final ModuleScript moduleScript = getModule(cx, id, uri);
260+
final ModuleScript moduleScript = getModule(cx, id, uri, base);
260261
if (sandboxed && !moduleScript.isSandboxed()) {
261262
throw ScriptRuntime.throwError(cx, nativeScope, "Module \""
262263
+ id + "\" is not contained in sandbox.");
@@ -353,10 +354,10 @@ private static void defineReadOnlyProperty(ScriptableObject obj,
353354
ScriptableObject.PERMANENT);
354355
}
355356

356-
private ModuleScript getModule(Context cx, String id, URI uri) {
357+
private ModuleScript getModule(Context cx, String id, URI uri, URI base) {
357358
try {
358359
final ModuleScript moduleScript =
359-
moduleScriptProvider.getModuleScript(cx, id, uri, paths);
360+
moduleScriptProvider.getModuleScript(cx, id, uri, base, paths);
360361
if (moduleScript == null) {
361362
throw ScriptRuntime.throwError(cx, nativeScope, "Module \""
362363
+ id + "\" not found.");

‎src/org/mozilla/javascript/commonjs/module/provider/CachingModuleScriptProviderBase.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,13 @@ protected CachingModuleScriptProviderBase(
5858
}
5959

6060
public ModuleScript getModuleScript(Context cx, String moduleId,
61-
URI moduleUri, Scriptable paths) throws Exception
61+
URI moduleUri, URI baseUri, Scriptable paths) throws Exception
6262
{
6363
final CachedModuleScript cachedModule1 = getLoadedModule(moduleId);
6464
final Object validator1 = getValidator(cachedModule1);
6565
final ModuleSource moduleSource = (moduleUri == null)
6666
? moduleSourceProvider.loadSource(moduleId, paths, validator1)
67-
: moduleSourceProvider.loadSource(moduleUri, validator1);
67+
: moduleSourceProvider.loadSource(moduleUri, baseUri, validator1);
6868
if(moduleSource == ModuleSourceProvider.NOT_MODIFIED) {
6969
return cachedModule1.getModule();
7070
}

‎src/org/mozilla/javascript/commonjs/module/provider/ModuleSourceProvider.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public interface ModuleSourceProvider
1616
{
1717
/**
1818
* A special return value for {@link #loadSource(String, Scriptable,
19-
* Object)} and {@link #loadSource(URI, Object)} that signifies that the
19+
* Object)} and {@link #loadSource(URI, URI, Object)} that signifies that the
2020
* cached representation is still valid according to the passed validator.
2121
*/
2222
public static final ModuleSource NOT_MODIFIED = new ModuleSource(null,
@@ -72,6 +72,8 @@ public ModuleSource loadSource(String moduleId, Scriptable paths, Object validat
7272
* validator for it, and a security domain, where applicable.
7373
* @param uri the absolute URI from which to load the module source, but
7474
* without an extension such as ".js".
75+
* @param baseUri the module path base URI from which <code>uri</code>
76+
* was derived.
7577
* @param validator a validator for an existing loaded and cached module.
7678
* This will either be null, or an object that this source provider
7779
* returned earlier as part of a {@link ModuleSource}. It can be used to
@@ -83,6 +85,6 @@ public ModuleSource loadSource(String moduleId, Scriptable paths, Object validat
8385
* @throws IOException if there was an I/O problem reading the script
8486
* @throws URISyntaxException if the final URI could not be constructed
8587
*/
86-
public ModuleSource loadSource(URI uri, Object validator)
88+
public ModuleSource loadSource(URI uri, URI baseUri, Object validator)
8789
throws IOException, URISyntaxException;
8890
}

‎src/org/mozilla/javascript/commonjs/module/provider/ModuleSourceProviderBase.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ public ModuleSource loadSource(String moduleId, Scriptable paths,
4949
return loadFromFallbackLocations(moduleId, validator);
5050
}
5151

52-
public ModuleSource loadSource(URI uri, Object validator)
52+
public ModuleSource loadSource(URI uri, URI base, Object validator)
5353
throws IOException, URISyntaxException {
54-
return loadFromUri(uri, null, validator);
54+
return loadFromUri(uri, base, validator);
5555
}
5656

5757
private ModuleSource loadFromPathArray(String moduleId,

‎src/org/mozilla/javascript/commonjs/module/provider/MultiModuleScriptProvider.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ public MultiModuleScriptProvider(Iterable<? extends ModuleScriptProvider> provid
3232
}
3333

3434
public ModuleScript getModuleScript(Context cx, String moduleId, URI uri,
35-
Scriptable paths) throws Exception {
35+
URI base, Scriptable paths) throws Exception {
3636
for (ModuleScriptProvider provider : providers) {
3737
final ModuleScript script = provider.getModuleScript(cx, moduleId,
38-
uri, paths);
38+
uri, base, paths);
3939
if(script != null) {
4040
return script;
4141
}

‎src/org/mozilla/javascript/commonjs/module/provider/SoftCachingModuleScriptProvider.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public SoftCachingModuleScriptProvider(
4848

4949
@Override
5050
public ModuleScript getModuleScript(Context cx, String moduleId,
51-
URI uri, Scriptable paths)
51+
URI uri, URI base, Scriptable paths)
5252
throws Exception
5353
{
5454
// Overridden to clear the reference queue before retrieving the
@@ -60,7 +60,7 @@ public ModuleScript getModuleScript(Context cx, String moduleId,
6060
}
6161
scripts.remove(ref.getModuleId(), ref);
6262
}
63-
return super.getModuleScript(cx, moduleId, uri, paths);
63+
return super.getModuleScript(cx, moduleId, uri, base, paths);
6464
}
6565

6666
@Override

‎testsrc/org/mozilla/javascript/tests/commonjs/module/modules.jar‎

-5.88 KB
Binary file not shown.

‎testsrc/org/mozilla/javascript/tests/commonjs/module/testRelativeId.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ function testRelativeRequire() {
99
}
1010
}
1111
testRelativeRequire();
12-
assert.ok(assert === require("assert"));
13-
assert.ok(assert === require("x/modx").assertThroughX);
14-
assert.ok(assert === require("x/y/mody").assertThroughXAndY);
15-
assert.ok(assert === require("x/y/mody").assertThroughY);
12+
assert.strictEqual(assert, require("assert"));
13+
assert.strictEqual(assert, require("x/y/mody").assertThroughXAndY);
14+
assert.strictEqual(assert, require("x/y/mody").assertThroughY);
15+
assert.strictEqual(assert, require("x/modx").assertThroughX);
16+
assert.strictEqual(require("x/y/mody").modz, require("x/modx").modz);
17+
assert.strictEqual(require("x/y/mody").modz.success, true);

‎testsrc/org/mozilla/javascript/tests/commonjs/module/x/modx.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
exports.assertThroughX = require("../assert");
1+
exports.assertThroughX = require("../assert");
2+
exports.modz = require("./modz");

0 commit comments

Comments
 (0)