Merge pull request #1336 from solid/fix/1318

Fix/1318  .mp3 files uploaded with dataBrowser as .mp3$.mp3

Author: jaxoncreed

lib/resource-mapper.js

@@ -33,7 +33,7 @@ class ResourceMapper {
     this._defaultContentType = defaultContentType
     this._types = { ...types, ...overrideTypes }
     this._indexFilename = indexFilename
-    this._indexContentType = this._getContentTypeByExtension(indexFilename)
+    this._indexContentType = this._getContentTypeFromExtension(indexFilename)
 
     // If the host needs to be replaced on every call, pre-split the root URL
     if (includeHost) {
@@ -79,7 +79,7 @@ class ResourceMapper {
     // Determine the URL by chopping off everything after the dollar sign
     const pathname = this._removeDollarExtension(path)
     const url = `${this.resolveUrl(hostname)}${encodeURI(pathname)}`
-    return { url, contentType: this._getContentTypeByExtension(path) }
+    return { url, contentType: this._getContentTypeFromExtension(path) }
   }
 
   // Maps the request for a given resource and representation format to a server file
@@ -94,7 +94,7 @@ class ResourceMapper {
     let isFolder = filePath.endsWith('/')
     let isIndex = searchIndex && filePath.endsWith('/')
 
-    // Create the path for a new ressource
+    // Create the path for a new resource
     let path
     if (createIfNotExists) {
       path = filePath
@@ -106,8 +106,8 @@ class ResourceMapper {
         path += this._indexFilename
       }
       // If the extension is not correct for the content type, append the correct extension
-      if (!isFolder && this._getContentTypeByExtension(path) !== contentType) {
-        path += `$${contentType in extensions ? `.${extensions[contentType][0]}` : '.unknown'}`
+      if (!isFolder) {
+        path = this._addContentTypeExtension(path, contentType)
       }
     // Determine the path of an existing file
     } else {
@@ -136,7 +136,7 @@ class ResourceMapper {
         }
       }
       path = `${folder}${match}`
-      contentType = this._getContentTypeByExtension(match)
+      contentType = this._getContentTypeFromExtension(match)
     }
     return { path, contentType: contentType || this._defaultContentType }
   }
@@ -157,11 +157,25 @@ class ResourceMapper {
   }
 
   // Gets the expected content type based on the extension of the path
-  _getContentTypeByExtension (path) {
+  _getContentTypeFromExtension (path) {
     const extension = /\.([^/.]+)$/.exec(path)
     return extension && this._types[extension[1].toLowerCase()] || this._defaultContentType
   }
 
+  // Appends an extension for the specific content type, if needed
+  _addContentTypeExtension (path, contentType) {
+    // If we would guess the wrong content type from the extension, try appending a better one
+    const contentTypeFromExtension = this._getContentTypeFromExtension(path)
+    if (contentTypeFromExtension !== contentType) {
+      // Some extensions fit multiple content types, so only switch if there's an improvement
+      const newExtension = contentType in extensions ? extensions[contentType][0] : 'unknown'
+      if (this._types[newExtension] !== contentTypeFromExtension) {
+        path += `$.${newExtension}`
+      }
+    }
+    return path
+  }
+
   // Removes possible trailing slashes from a path
   _removeTrailingSlash (path) {
     return path.replace(/\/+$/, '')

test/unit/resource-mapper-test.js

@@ -119,6 +119,17 @@ describe('ResourceMapper', () => {
         contentType: 'text/n3'
       })
 
+    itMapsUrl(mapper, 'a URL with a file extension having more than one possible content type',
+      {
+        url: 'http://localhost/space/foo.mp3',
+        contentType: 'audio/mp3',
+        createIfNotExists: true
+      },
+      {
+        path: `${rootPath}space/foo.mp3`,
+        contentType: 'audio/mp3'
+      })
+
     // GET/HEAD/POST/DELETE/PATCH base cases
 
     itMapsUrl(mapper, 'a URL of a non-existing file',