Stricter checks for paths and project/asset/version names.

- Prevent any path or name from containing Windows backslashes.
- More informative error messages for invalid names.
- Paths cannot be empty.
- Paths cannot contain repeated slashes, e.g., a//b is not allowed.
- Paths should not start or end with a slash.

src/upload.js

@@ -24,10 +24,24 @@ function splitByUploadType(files) {
         if (!("path" in f) || typeof f.path != "string") {
             throw new http.HttpError("'files.path' should be a string", 400);
         }
+
         let fname = f.path;
+        if (fname.length == 0) {
+            throw new http.HttpError("'files.path' cannot be empty", 400);
+        }
         if (misc.isInternalPath(fname)) {
             throw new http.HttpError("components of 'files.path' cannot start with '..'", 400);
         }
+        if (fname.endsWith("/") || fname.startsWith("/")) {
+            throw new http.HttpError("'files.path' cannot start or end with '/'");
+        }
+        if (fname.indexOf("//") >= 0) {
+            throw new http.HttpError("'files.path' cannot contain repeated '/'");
+        }
+        if (fname.indexOf("\\") >= 0) {
+            throw new http.HttpError("'files.path' cannot contain '\\'");
+        }
+
         if (all_paths.has(fname)) {
             throw new http.HttpError("duplicated value '" + fname + "' in 'files.path'", 400);
         }
@@ -185,8 +199,16 @@ async function checkLinks(linked, project, asset, version, env, manifest_cache)
     return linked_details;
 }
 
-function isBadName(name) {
-    return name.indexOf("/") >= 0 || name.startsWith("..") || name.length == 0;
+function checkBadName(name, type) {
+    if (name.length == 0) {
+        throw new http.HttpError(type + " name cannot be empty", 400);
+    }
+    if (name.indexOf("/") >= 0 || name.indexOf('\\') >= 0) {
+        throw new http.HttpError(type + " name cannot contain '/' or '\\'", 400);
+    }
+    if (name.startsWith("..")) {
+        throw new http.HttpError(type + " name cannot start with '..'", 400);
+    }
 }
 
 const pending_name = "~pending_on_complete_only";
@@ -196,15 +218,9 @@ export async function initializeUploadHandler(request, env, nonblockers) {
     let asset = decodeURIComponent(request.params.asset);
     let version = decodeURIComponent(request.params.version);
 
-    if (isBadName(project)) {
-        throw new http.HttpError("project name cannot contain '/', start with '..', or be empty", 400);
-    }
-    if (isBadName(asset)) {
-        throw new http.HttpError("asset name cannot contain '/', start with '..', or be empty", 400);
-    }
-    if (isBadName(version)) {
-        throw new http.HttpError("version name cannot contain '/', start with '..', or be empty", 400);
-    }
+    checkBadName(project, "project");
+    checkBadName(asset, "asset");
+    checkBadName(version, "version");
 
     let body = await http.bodyToJson(request);
     if (!misc.isJsonObject(body)) {

tests/upload.test.js

@@ -60,12 +60,24 @@ test("initializeUploadHandler throws the right errors related to formatting", as
     req = create_req({ files: [ { "path": 2 } ] });
     await setup.expectError(upload.initializeUploadHandler(req, env, nb), "'files.path' should be a string");
 
+    req = create_req({ files: [ { "path": "" } ] });
+    await setup.expectError(upload.initializeUploadHandler(req, env, nb), "cannot be empty");
+
     req = create_req({ files: [ { "path": "..asdasd" } ] });
     await setup.expectError(upload.initializeUploadHandler(req, env, nb), "cannot start with");
 
     req = create_req({ files: [ { "path": "yay/..asdasd" } ] });
     await setup.expectError(upload.initializeUploadHandler(req, env, nb), "cannot start with");
 
+    req = create_req({ files: [ { "path": "/asd" } ] });
+    await setup.expectError(upload.initializeUploadHandler(req, env, nb), "cannot start or end with '/'");
+
+    req = create_req({ files: [ { "path": "asdad//asdasd" } ] });
+    await setup.expectError(upload.initializeUploadHandler(req, env, nb), "cannot contain repeated '/'");
+
+    req = create_req({ files: [ { "path": "asdad\\asdasd" } ] });
+    await setup.expectError(upload.initializeUploadHandler(req, env, nb), "cannot contain '\\'");
+
     req = create_req({ files: [ { "path": "asdasd" } ] });
     await setup.expectError(upload.initializeUploadHandler(req, env, nb), "'files.type' should be a string");
 
@@ -130,6 +142,27 @@ test("initializeUploadHandler throws the right errors for permission-related err
     }
 })
 
+test("initializeUploadHandler throws the right errors for invalid project names", async () => {
+    const env = getMiniflareBindings();
+    await setup.createMockProject("test-upload", env);
+    let options = { method: "POST", body: JSON.stringify({ files: [] }) };
+
+    let req = new Request("http://localhost", options);
+    req.params = { project: "test/upload", asset: "palms", version: "test" };
+    let nb = [];
+    req.headers.append("Authorization", "Bearer " + setup.mockTokenOwner);
+    await setup.expectError(upload.initializeUploadHandler(req, env, nb), "project name cannot contain");
+
+    req.params = { project: "test-upload", asset: "foo\\bar", version: "test" };
+    await setup.expectError(upload.initializeUploadHandler(req, env, nb), "asset name cannot contain");
+
+    req.params = { project: "test-upload", asset: "foo-bar", version: "..test" };
+    await setup.expectError(upload.initializeUploadHandler(req, env, nb), "version name cannot start with");
+
+    req.params = { project: "test-upload", asset: "foo-bar", version: "" };
+    await setup.expectError(upload.initializeUploadHandler(req, env, nb), "version name cannot be empty");
+})
+
 test("initializeUploadHandler works correctly for simple uploads", async () => {
     const env = getMiniflareBindings();
     await setup.createMockProject("test-upload", env);