From 1d58ac4d793dd1842ea04f50b994ee2b455103b7 Mon Sep 17 00:00:00 2001
From: Dale Glass <dale@daleglass.net>
Date: Wed, 23 Sep 2020 23:33:22 +0200
Subject: [PATCH] Sanitize the URL further by removing leading slashes and any
 duplicated ones

---
 libraries/networking/src/ExternalResource.cpp | 23 +++++++++++++++++--
 libraries/script-engine/src/ScriptEngine.h    |  3 +--
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/libraries/networking/src/ExternalResource.cpp b/libraries/networking/src/ExternalResource.cpp
index e2b6a40682..a760e3554f 100644
--- a/libraries/networking/src/ExternalResource.cpp
+++ b/libraries/networking/src/ExternalResource.cpp
@@ -49,13 +49,32 @@ QUrl ExternalResource::getQUrl(Bucket bucket, const QUrl& path) {
         base = _bucketBases[bucket];
     }
 
-    QUrl merged = base.resolved(path).adjusted(QUrl::NormalizePathSegments);
+    QUrl filteredPath = path;
+    QString pathQString = filteredPath.path();
+
+    // De-duplicate URL separators, since S3 doesn't actually have subdirectories, and treats a '/' as a part
+    // of the filename. This means that "/dir/file.txt" and "/dir//file.txt" are not equivalent on S3.
+    while(pathQString.contains("//")) {
+        pathQString = pathQString.replace("//", "/");
+    }
+
+    // If a path starts with a / it would have the effect of overriding any path specified in the bucket.
+    // We remove it, ensuring that getQUrl(Bucket.Assets, "/file.txt") and getQUrl(Bucket.Assets, "file.txt")
+    // are equivalent.
+    if (pathQString.startsWith("/")) {
+        pathQString.remove(0,1);
+    }
+
+    filteredPath.setPath(pathQString);
+
+    QUrl merged = base.resolved(filteredPath).adjusted(QUrl::NormalizePathSegments);
 
     if ( merged.isValid() ) {
         qCDebug(external_resource) << "External resource resolved to " << merged;
     } else {
         qCCritical(external_resource) << "External resource resolved to invalid URL " << merged << "; Error " 
-                                      << merged.errorString() << "; base = " << base << "; path = " << path;
+                                      << merged.errorString() << "; base = " << base << "; path = " << path
+                                      << "; filtered path = " << filteredPath;
     }
 
     return merged;
diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h
index f393177c52..fff1cab00a 100644
--- a/libraries/script-engine/src/ScriptEngine.h
+++ b/libraries/script-engine/src/ScriptEngine.h
@@ -693,8 +693,7 @@ public:
      * @param {string} path - The path within the external resource bucket where the asset is located. 
      *     <p>Normally, this should start with a path or filename to be appended to the bucket URL.
      *     Alternatively, it can be a relative path starting with <code>./</code> or <code>../</code>, to navigate within the 
-     *     resource bucket's URL. Or it can be an absolute path starting with <code>/</code>, in which case the bucket's path
-     *     is discarded when calculating the asset's URL.</p>
+     *     resource bucket's URL.</p>
      * @Returns {string} The URL of an external asset.
      * @example <caption>Report the URL of a default particle.</caption>
      * print(Script.getExternalPath(Script.ExternalPaths.Assets, "Bazaar/Assets/Textures/Defaults/Interface/default_particle.png"));