Web extension resources return incorrect MIME type

Use [Content_Types].xml to set FileResource contentType Add test case for default content type: application/octet-stream
eclipse · Sep 27, 2022 · d9e2b5e · d9e2b5e
1 parent f956110
commit d9e2b5e
Show file tree

Hide file tree

Showing 31 changed files with 753 additions and 98 deletions.
diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java b/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java
@@ -9,10 +9,7 @@
  ********************************************************************************/
 package org.eclipse.openvsx;
 
-import java.io.EOFException;
-import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
+import java.io.*;
 import java.util.*;
 import java.util.function.Consumer;
 import java.util.regex.Pattern;
@@ -40,6 +37,11 @@
 import org.slf4j.LoggerFactory;
 import org.springframework.data.util.Pair;
 import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.xml.sax.SAXException;
+
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
 
 /**
  * Processes uploaded extension files and extracts their metadata.
@@ -301,6 +303,7 @@ public List<FileResource> getResources(ExtensionVersion extVersion) {
 
     public List<FileResource> getResources(ExtensionVersion extVersion, List<String> excludes) {
         var resources = new ArrayList<FileResource>();
+        var contentTypes = loadContentTypes(resources);
         if(!excludes.contains(FileResource.RESOURCE)) {
             resources.addAll(getAllResources(extVersion).collect(Collectors.toList()));
         }
@@ -335,7 +338,9 @@ public List<FileResource> getResources(ExtensionVersion extVersion, List<String>
                 resources.add(icon);
         }
 
-        return resources;
+        return resources.stream()
+                .map(resource -> setContentType(resource, contentTypes))
+                .collect(Collectors.toList());
     }
 
     public void processEachResource(ExtensionVersion extVersion, Consumer<FileResource> processor) {
@@ -461,6 +466,48 @@ public FileResource getLicense(ExtensionVersion extVersion) {
         return license;
     }
 
+    private Map<String, String> loadContentTypes(List<FileResource> resources) {
+        var contentTypes = resources.stream()
+                .filter(r -> r.getName().equals("[Content_Types].xml"))
+                .findFirst()
+                .map(FileResource::getContent)
+                .map(this::parseContentTypesXml)
+                .orElse(new HashMap<>());
+
+        contentTypes.putIfAbsent(".vsix", "application/zip");
+        return contentTypes;
+    }
+
+    private Map<String, String> parseContentTypesXml(byte[] content) {
+        try (var input = new ByteArrayInputStream(content)) {
+            var contentTypes = new HashMap<String, String>();
+            var document = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(input);
+            var elements = document.getDocumentElement().getElementsByTagName("Default");
+            for(var i = 0; i < elements.getLength(); i++) {
+                var element = elements.item(i);
+                var attributes = element.getAttributes();
+                var extension = attributes.getNamedItem("Extension").getTextContent();
+                var contentType = attributes.getNamedItem("ContentType").getTextContent();
+                contentTypes.put(extension, contentType);
+            }
+
+            return contentTypes;
+        } catch (IOException | ParserConfigurationException | SAXException e) {
+            logger.error("failed to read content types", e);
+            return null;
+        }
+    }
+
+    private FileResource setContentType(FileResource resource, Map<String, String> contentTypes) {
+        var resourceName = Optional.ofNullable(resource.getName()).orElse("");
+        var fileExtensionIndex = resourceName.lastIndexOf('.');
+        var fileExtension = fileExtensionIndex != -1 ? resourceName.substring(fileExtensionIndex) : "";
+        var contentType = contentTypes.getOrDefault(fileExtension, MediaType.APPLICATION_OCTET_STREAM_VALUE);
+
+        resource.setContentType(contentType);
+        return resource;
+    }
+
     private void detectLicense(byte[] content, ExtensionVersion extVersion) {
         if (Strings.isNullOrEmpty(extVersion.getLicense())) {
             var detection = new LicenseDetection();

diff --git a/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java b/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java
@@ -173,7 +173,7 @@ public ResponseEntity<byte[]> getFile(String namespace, String extensionName, St
         if (resource.getType().equals(DOWNLOAD))
             storageUtil.increaseDownloadCount(extVersion, resource);
         if (resource.getStorageType().equals(FileResource.STORAGE_DB)) {
-            var headers = storageUtil.getFileResponseHeaders(fileName);
+            var headers = storageUtil.getFileResponseHeaders(resource);
             return new ResponseEntity<>(resource.getContent(), headers, HttpStatus.OK);
         } else {
             return ResponseEntity.status(HttpStatus.FOUND)

diff --git a/server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java b/server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java
@@ -304,7 +304,7 @@ public ResponseEntity<byte[]> getAsset(
             storageUtil.increaseDownloadCount(extVersion, resource);
         }
         if (resource.getStorageType().equals(FileResource.STORAGE_DB)) {
-            var headers = storageUtil.getFileResponseHeaders(resource.getName());
+            var headers = storageUtil.getFileResponseHeaders(resource);
             return new ResponseEntity<>(resource.getContent(), headers, HttpStatus.OK);
         } else {
             return ResponseEntity.status(HttpStatus.FOUND)
@@ -416,7 +416,7 @@ private ResponseEntity<byte[]> browseFile(
             String version
     ) {
         if (resource.getStorageType().equals(FileResource.STORAGE_DB)) {
-            var headers = storageUtil.getFileResponseHeaders(resource.getName());
+            var headers = storageUtil.getFileResponseHeaders(resource);
             return new ResponseEntity<>(resource.getContent(), headers, HttpStatus.OK);
         } else {
             var namespace = new Namespace();

diff --git a/server/src/main/java/org/eclipse/openvsx/dto/FileResourceDTO.java b/server/src/main/java/org/eclipse/openvsx/dto/FileResourceDTO.java
@@ -11,8 +11,6 @@
 
 import org.eclipse.openvsx.entities.FileResource;
 
-import java.util.Objects;
-
 public class FileResourceDTO {
 
     private final long extensionVersionId;
@@ -23,11 +21,21 @@ public class FileResourceDTO {
     private final String type;
     private String storageType;
     private byte[] content;
-
-    public FileResourceDTO(long id, long extensionVersionId, String name, String type, String storageType, byte[] content) {
+    private String contentType;
+
+    public FileResourceDTO(
+            long id,
+            long extensionVersionId,
+            String name,
+            String type,
+            String storageType,
+            byte[] content,
+            String contentType
+    ) {
         this(id, extensionVersionId, name, type);
         this.storageType = storageType;
         this.content = content;
+        this.contentType = contentType;
     }
 
     public FileResourceDTO(long id, long extensionVersionId, String name, String type) {
@@ -73,6 +81,8 @@ public byte[] getContent() {
         return content;
     }
 
+    public String getContentType() { return contentType; }
+
     public boolean isWebResource() {
         return type.equals(FileResource.RESOURCE) && name.startsWith("extension/");
     }

diff --git a/server/src/main/java/org/eclipse/openvsx/entities/FileResource.java b/server/src/main/java/org/eclipse/openvsx/entities/FileResource.java
@@ -45,6 +45,8 @@ public class FileResource {
     @Basic(fetch = FetchType.LAZY)
     byte[] content;
 
+    String contentType;
+
     @Column(length = 32)
     String storageType;
 
@@ -89,6 +91,14 @@ public void setContent(byte[] content) {
 		this.content = content;
 	}
 
+	public String getContentType() {
+        return contentType;
+    }
+
+    public void setContentType(String contentType) {
+        this.contentType = contentType;
+    }
+
     public String getStorageType() {
         return storageType;
     }

diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/FileResourceDTORepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/FileResourceDTORepository.java
@@ -13,9 +13,6 @@
 import org.eclipse.openvsx.entities.FileResource;
 import org.jooq.DSLContext;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.data.jpa.repository.Query;
-import org.springframework.data.repository.Repository;
-import org.springframework.data.util.Streamable;
 import org.springframework.stereotype.Component;
 
 import java.util.Collection;
@@ -30,15 +27,28 @@ public class FileResourceDTORepository {
     DSLContext dsl;
 
     public List<FileResourceDTO> findAll(Collection<Long> extensionIds, Collection<String> types) {
-        return dsl.select(FILE_RESOURCE.ID, FILE_RESOURCE.EXTENSION_ID, FILE_RESOURCE.NAME, FILE_RESOURCE.TYPE)
+        return dsl.select(
+                    FILE_RESOURCE.ID,
+                    FILE_RESOURCE.EXTENSION_ID,
+                    FILE_RESOURCE.NAME,
+                    FILE_RESOURCE.TYPE
+                )
                 .from(FILE_RESOURCE)
                 .where(FILE_RESOURCE.EXTENSION_ID.in(extensionIds))
                 .and(FILE_RESOURCE.TYPE.in(types))
                 .fetchInto(FileResourceDTO.class);
     }
 
     public List<FileResourceDTO> findAllResources(long extVersionId, String prefix) {
-        return dsl.select(FILE_RESOURCE.ID, FILE_RESOURCE.EXTENSION_ID, FILE_RESOURCE.NAME, FILE_RESOURCE.TYPE, FILE_RESOURCE.STORAGE_TYPE, FILE_RESOURCE.CONTENT)
+        return dsl.select(
+                    FILE_RESOURCE.ID,
+                    FILE_RESOURCE.EXTENSION_ID,
+                    FILE_RESOURCE.NAME,
+                    FILE_RESOURCE.TYPE,
+                    FILE_RESOURCE.STORAGE_TYPE,
+                    FILE_RESOURCE.CONTENT,
+                    FILE_RESOURCE.CONTENT_TYPE
+                )
                 .from(FILE_RESOURCE)
                 .where(FILE_RESOURCE.TYPE.eq(FileResource.RESOURCE))
                 .and(FILE_RESOURCE.EXTENSION_ID.eq(extVersionId))

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/AzureBlobStorageService.java b/server/src/main/java/org/eclipse/openvsx/storage/AzureBlobStorageService.java
@@ -64,19 +64,21 @@ public void uploadFile(FileResource resource) {
                     + blobName + ": missing Azure blob service endpoint");
         }
 
-        uploadFile(resource.getContent(), resource.getName(), blobName);
+        uploadFile(resource, blobName);
     }
 
-    protected void uploadFile(byte[] content, String fileName, String blobName) {
+    protected void uploadFile(FileResource resource, String blobName) {
         var blobClient = getContainerClient().getBlobClient(blobName);
         var headers = new BlobHttpHeaders();
-        headers.setContentType(StorageUtil.getFileType(fileName).toString());
-        if (fileName.endsWith(".vsix")) {
-            headers.setContentDisposition("attachment; filename=\"" + fileName + "\"");
+        headers.setContentType(resource.getContentType());
+        if (resource.getName().endsWith(".vsix")) {
+            headers.setContentDisposition("attachment; filename=\"" + resource.getName() + "\"");
         } else {
-            var cacheControl = StorageUtil.getCacheControl(fileName);
+            var cacheControl = StorageUtil.getCacheControl(resource.getName());
             headers.setCacheControl(cacheControl.getHeaderValue());
         }
+
+        var content = resource.getContent();
         try (var dataStream = new ByteArrayInputStream(content)) {
             blobClient.upload(dataStream, content.length, true);
             blobClient.setHttpHeaders(headers);

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/GoogleCloudStorageService.java b/server/src/main/java/org/eclipse/openvsx/storage/GoogleCloudStorageService.java
@@ -64,19 +64,19 @@ public void uploadFile(FileResource resource) {
                     + objectId + ": missing Google bucket id");
         }
 
-        uploadFile(resource.getContent(), resource.getName(), objectId);
+        uploadFile(resource, objectId);
     }
 
-    protected void uploadFile(byte[] content, String fileName, String objectId) {
+    protected void uploadFile(FileResource resource, String objectId) {
         var blobInfoBuilder = BlobInfo.newBuilder(BlobId.of(bucketId, objectId))
-                .setContentType(StorageUtil.getFileType(fileName).toString());
-        if (fileName.endsWith(".vsix")) {
-            blobInfoBuilder.setContentDisposition("attachment; filename=\"" + fileName + "\"");
+                .setContentType(resource.getContentType());
+        if (resource.getName().endsWith(".vsix")) {
+            blobInfoBuilder.setContentDisposition("attachment; filename=\"" + resource.getName() + "\"");
         } else {
-            var cacheControl = StorageUtil.getCacheControl(fileName);
+            var cacheControl = StorageUtil.getCacheControl(resource.getName());
             blobInfoBuilder.setCacheControl(cacheControl.getHeaderValue());
         }
-        getStorage().create(blobInfoBuilder.build(), content);
+        getStorage().create(blobInfoBuilder.build(), resource.getContent());
     }
 
     @Override

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/StorageUtil.java b/server/src/main/java/org/eclipse/openvsx/storage/StorageUtil.java
@@ -11,26 +11,13 @@
 package org.eclipse.openvsx.storage;
 
 import org.springframework.http.CacheControl;
-import org.springframework.http.MediaType;
 
-import java.net.URLConnection;
 import java.util.concurrent.TimeUnit;
 
 class StorageUtil {
 
     private StorageUtil(){}
 
-    static MediaType getFileType(String fileName) {
-        if (fileName.endsWith(".vsix"))
-            return MediaType.APPLICATION_OCTET_STREAM;
-        if (fileName.endsWith(".json"))
-            return MediaType.APPLICATION_JSON;
-        var contentType = URLConnection.guessContentTypeFromName(fileName);
-        if (contentType != null)
-            return MediaType.parseMediaType(contentType);
-        return MediaType.TEXT_PLAIN;
-    }
-
     static CacheControl getCacheControl(String fileName) {
         // Files are requested with a version string in the URL, so their content cannot change
         return CacheControl.maxAge(30, TimeUnit.DAYS).cachePublic();

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/StorageUtilService.java b/server/src/main/java/org/eclipse/openvsx/storage/StorageUtilService.java
@@ -10,6 +10,8 @@
 package org.eclipse.openvsx.storage;
 
 import com.google.common.base.Strings;
+
+import org.eclipse.openvsx.dto.FileResourceDTO;
 import com.google.common.collect.Maps;
 import org.eclipse.openvsx.entities.ExtensionVersion;
 import org.eclipse.openvsx.entities.FileResource;
@@ -19,6 +21,7 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.MediaType;
 import org.springframework.stereotype.Component;
 
 import javax.transaction.Transactional;
@@ -170,9 +173,17 @@ public void increaseDownloadCount(ExtensionVersion extVersion, FileResource reso
         downloadCountService.increaseDownloadCount(extVersion, resource, List.of(TimeUtil.getCurrentUTC()));
     }
 
-    public HttpHeaders getFileResponseHeaders(String fileName) {
+    public HttpHeaders getFileResponseHeaders(FileResource resource) {
+        return getFileResponseHeaders(resource.getName(), resource.getContentType());
+    }
+
+    public HttpHeaders getFileResponseHeaders(FileResourceDTO resource) {
+        return getFileResponseHeaders(resource.getName(), resource.getContentType());
+    }
+
+    private HttpHeaders getFileResponseHeaders(String fileName, String contentType) {
         var headers = new HttpHeaders();
-        headers.setContentType(StorageUtil.getFileType(fileName));
+        headers.setContentType(MediaType.parseMediaType(contentType));
         if (fileName.endsWith(".vsix")) {
             headers.add("Content-Disposition", "attachment; filename=\"" + fileName + "\"");
         } else {

diff --git a/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/Indexes.java b/server/src/main/jooq-gen/org/eclipse/openvsx/jooq/Indexes.java