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 Add migration to set FileResource.contentType Set contentType for RESOURCE type Prepend dot ('.') if extension doesn't start with a dot.
eclipse · Dec 16, 2022 · 9f36399 · 9f36399
1 parent f061c72
commit 9f36399
Show file tree

Hide file tree

Showing 39 changed files with 886 additions and 142 deletions.
diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java b/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java
@@ -9,6 +9,7 @@
  ********************************************************************************/
 package org.eclipse.openvsx;
 
+import java.io.ByteArrayInputStream;
 import java.io.EOFException;
 import java.io.IOException;
 import java.nio.file.Path;
@@ -36,6 +37,11 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.data.util.Pair;
+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.
@@ -282,17 +288,21 @@ private List<String> getEngines(JsonNode node) {
     }
 
     public List<FileResource> getFileResources(ExtensionVersion extVersion) {
-        var resources = new ArrayList<FileResource>();
+        var contentTypes = loadContentTypes();
         var mappers = List.<Function<ExtensionVersion, FileResource>>of(
                 this::getManifest, this::getReadme, this::getChangelog, this::getLicense, this::getIcon
         );
 
-        mappers.forEach(mapper -> Optional.of(extVersion).map(mapper).ifPresent(resources::add));
-        return resources;
+        return mappers.stream()
+                .map(mapper -> mapper.apply(extVersion))
+                .filter(Objects::nonNull)
+                .map(resource -> setContentType(resource, contentTypes))
+                .collect(Collectors.toList());
     }
 
     public void processEachResource(ExtensionVersion extVersion, Consumer<FileResource> processor) {
         readInputStream();
+        var contentTypes = loadContentTypes();
         zipFile.stream()
                 .filter(zipEntry -> !zipEntry.isDirectory())
                 .map(zipEntry -> {
@@ -311,6 +321,7 @@ public void processEachResource(ExtensionVersion extVersion, Consumer<FileResour
                     resource.setName(zipEntry.getName());
                     resource.setType(FileResource.RESOURCE);
                     resource.setContent(bytes);
+                    setContentType(resource, contentTypes);
                     return resource;
                 })
                 .filter(Objects::nonNull)
@@ -413,6 +424,46 @@ public FileResource getLicense(ExtensionVersion extVersion) {
         return license;
     }
 
+    private Map<String, String> loadContentTypes() {
+        var bytes = ArchiveUtil.readEntry(zipFile, "[Content_Types].xml");
+        var contentTypes = parseContentTypesXml(bytes);
+        contentTypes.putIfAbsent(".vsix", "application/zip");
+        return contentTypes;
+    }
+
+    private Map<String, String> parseContentTypesXml(byte[] content) {
+        var contentTypes = new HashMap<String, String>();
+        try (var input = new ByteArrayInputStream(content)) {
+            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();
+                if(!extension.startsWith(".")) {
+                    extension = "." + extension;
+                }
+
+                var contentType = attributes.getNamedItem("ContentType").getTextContent();
+                contentTypes.put(extension, contentType);
+            }
+        } catch (IOException | ParserConfigurationException | SAXException e) {
+            logger.error("failed to read content types", e);
+            contentTypes.clear();
+        }
+
+        return contentTypes;
+    }
+
+    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/adapter/LocalVSCodeService.java b/server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java
@@ -404,7 +404,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/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;
 
@@ -88,6 +90,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/migration/ExtractResourcesJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/migration/ExtractResourcesJobRequestHandler.java
@@ -10,6 +10,7 @@
 package org.eclipse.openvsx.migration;
 
 import org.eclipse.openvsx.ExtensionProcessor;
+import org.eclipse.openvsx.entities.ExtensionVersion;
 import org.jobrunr.jobs.annotations.Job;
 import org.jobrunr.jobs.context.JobRunrDashboardLogger;
 import org.jobrunr.jobs.lambdas.JobRequestHandler;
@@ -32,11 +33,11 @@ public class ExtractResourcesJobRequestHandler implements JobRequestHandler<Migr
     @Override
     @Job(name = "Extract resources from published extension version", retries = 3)
     public void run(MigrationJobRequest jobRequest) throws Exception {
-        var extVersion = service.getExtension(jobRequest.getEntityId());
+        var extVersion = migrations.find(jobRequest, ExtensionVersion.class);
         logger.info("Extracting resources for: {}.{}-{}@{}", extVersion.getExtension().getNamespace().getName(), extVersion.getExtension().getName(), extVersion.getVersion(), extVersion.getTargetPlatform());
 
         service.deleteResources(extVersion);
-        var entry = service.getDownload(extVersion);
+        var entry = migrations.getDownload(extVersion);
         var extensionFile = migrations.getExtensionFile(entry);
         var download = entry.getKey();
         try(var extProcessor = new ExtensionProcessor(extensionFile)) {

diff --git a/server/src/main/java/org/eclipse/openvsx/migration/ExtractResourcesJobService.java b/server/src/main/java/org/eclipse/openvsx/migration/ExtractResourcesJobService.java
@@ -12,20 +12,11 @@
 import org.eclipse.openvsx.entities.ExtensionVersion;
 import org.eclipse.openvsx.entities.FileResource;
 import org.eclipse.openvsx.repositories.RepositoryService;
-import org.eclipse.openvsx.storage.AzureBlobStorageService;
-import org.eclipse.openvsx.storage.GoogleCloudStorageService;
-import org.eclipse.openvsx.storage.IStorageService;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.http.HttpMethod;
-import org.springframework.retry.annotation.Retryable;
 import org.springframework.stereotype.Component;
-import org.springframework.web.client.RestTemplate;
 
 import javax.persistence.EntityManager;
 import javax.transaction.Transactional;
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
 import java.util.AbstractMap;
 import java.util.Map;
 
@@ -35,22 +26,9 @@ public class ExtractResourcesJobService {
     @Autowired
     RepositoryService repositories;
 
-    @Autowired
-    RestTemplate restTemplate;
-
     @Autowired
     EntityManager entityManager;
 
-    @Autowired
-    AzureBlobStorageService azureStorage;
-
-    @Autowired
-    GoogleCloudStorageService googleStorage;
-
-    public ExtensionVersion getExtension(long entityId) {
-        return entityManager.find(ExtensionVersion.class, entityId);
-    }
-
     @Transactional
     public void deleteResources(ExtensionVersion extVersion) {
         repositories.deleteFileResources(extVersion, "resource");
@@ -61,13 +39,6 @@ public void deleteWebResources(ExtensionVersion extVersion) {
         repositories.deleteFileResources(extVersion, "web-resource");
     }
 
-    @Transactional
-    public Map.Entry<FileResource, byte[]> getDownload(ExtensionVersion extVersion) {
-        var download = repositories.findFileByType(extVersion, FileResource.DOWNLOAD);
-        var content = download.getStorageType().equals(FileResource.STORAGE_DB) ? download.getContent() : null;
-        return new AbstractMap.SimpleEntry<>(download, content);
-    }
-
     @Transactional
     public void persistResource(FileResource resource) {
         entityManager.persist(resource);

diff --git a/server/src/main/java/org/eclipse/openvsx/migration/MigrationRunner.java b/server/src/main/java/org/eclipse/openvsx/migration/MigrationRunner.java
@@ -41,6 +41,7 @@ public void runMigrations(ApplicationStartedEvent event) {
         extractResourcesMigration();
         setPreReleaseMigration();
         renameDownloadsMigration();
+        setContentTypeMigration();
     }
 
     private void extractResourcesMigration() {
@@ -61,6 +62,12 @@ private void renameDownloadsMigration() {
         repositories.findNotMigratedRenamedDownloads().forEach(item -> enqueueJob(jobName, handler, item));
     }
 
+    private void setContentTypeMigration() {
+        var jobName = "SetContentTypeMigration";
+        var handler = SetContentTypeJobRequestHandler.class;
+        repositories.findNotMigratedContentTypes().forEach(item -> enqueueJob(jobName, handler, item));
+    }
+
     private void enqueueJob(String jobName, Class<? extends JobRequestHandler<MigrationJobRequest>> handler, MigrationItem item) {
         var jobIdText = jobName + "::itemId=" + item.getId();
         var jobId = UUID.nameUUIDFromBytes(jobIdText.getBytes(StandardCharsets.UTF_8));

diff --git a/server/src/main/java/org/eclipse/openvsx/migration/MigrationService.java b/server/src/main/java/org/eclipse/openvsx/migration/MigrationService.java
@@ -9,7 +9,9 @@
  * ****************************************************************************** */
 package org.eclipse.openvsx.migration;
 
+import org.eclipse.openvsx.entities.ExtensionVersion;
 import org.eclipse.openvsx.entities.FileResource;
+import org.eclipse.openvsx.repositories.RepositoryService;
 import org.eclipse.openvsx.storage.AzureBlobStorageService;
 import org.eclipse.openvsx.storage.GoogleCloudStorageService;
 import org.eclipse.openvsx.storage.IStorageService;
@@ -19,14 +21,23 @@
 import org.springframework.stereotype.Component;
 import org.springframework.web.client.RestTemplate;
 
+import javax.persistence.EntityManager;
+import javax.transaction.Transactional;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.AbstractMap;
 import java.util.Map;
 
 @Component
 public class MigrationService {
 
+    @Autowired
+    EntityManager entityManager;
+
+    @Autowired
+    RepositoryService repositories;
+
     @Autowired
     RestTemplate restTemplate;
 
@@ -36,6 +47,17 @@ public class MigrationService {
     @Autowired
     GoogleCloudStorageService googleStorage;
 
+    public <T> T find(MigrationJobRequest jobRequest, Class<T> clazz) {
+        return entityManager.find(clazz, jobRequest.getEntityId());
+    }
+
+    @Transactional
+    public Map.Entry<FileResource, byte[]> getDownload(ExtensionVersion extVersion) {
+        var download = repositories.findFileByType(extVersion, FileResource.DOWNLOAD);
+        var content = download.getStorageType().equals(FileResource.STORAGE_DB) ? download.getContent() : null;
+        return new AbstractMap.SimpleEntry<>(download, content);
+    }
+
     @Retryable
     public Path getExtensionFile(Map.Entry<FileResource, byte[]> entry) {
         Path extensionFile;

diff --git a/server/src/main/java/org/eclipse/openvsx/migration/RenameDownloadsJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/migration/RenameDownloadsJobRequestHandler.java
@@ -9,6 +9,7 @@
  * ****************************************************************************** */
 package org.eclipse.openvsx.migration;
 
+import org.eclipse.openvsx.entities.FileResource;
 import org.jobrunr.jobs.lambdas.JobRequestHandler;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -20,7 +21,7 @@
 @Component
 public class RenameDownloadsJobRequestHandler  implements JobRequestHandler<MigrationJobRequest> {
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(RenameDownloadsJobRequestHandler.class);
+    protected final Logger logger = LoggerFactory.getLogger(OrphanNamespaceMigration.class);
 
     @Autowired
     MigrationService migrations;
@@ -30,14 +31,14 @@ public class RenameDownloadsJobRequestHandler  implements JobRequestHandler<Migr
 
     @Override
     public void run(MigrationJobRequest jobRequest) throws Exception {
-        var download = service.getResource(jobRequest);
+        var download = migrations.find(jobRequest, FileResource.class);
         var name = service.getNewBinaryName(download);
         if(download.getName().equals(name)) {
             // names are the same, nothing to do
             return;
         }
 
-        LOGGER.info("Renaming download {}", download.getName());
+        logger.info("Renaming download {}", download.getName());
         var content = service.getContent(download);
         var extensionFile = migrations.getExtensionFile(new AbstractMap.SimpleEntry<>(download, content));
         var newDownload = service.cloneResource(download, name);
@@ -46,6 +47,6 @@ public void run(MigrationJobRequest jobRequest) throws Exception {
 
         download.setName(name);
         service.updateResource(download);
-        LOGGER.info("Updated download name to: {}", name);
+        logger.info("Updated download name to: {}", name);
     }
 }
diff --git a/server/src/main/java/org/eclipse/openvsx/migration/RenameDownloadsService.java b/server/src/main/java/org/eclipse/openvsx/migration/RenameDownloadsService.java
@@ -54,10 +54,6 @@ public FileResource cloneResource(FileResource resource, String name) {
         return clone;
     }
 
-    public FileResource getResource(MigrationJobRequest jobRequest) {
-        return entityManager.find(FileResource.class, jobRequest.getEntityId());
-    }
-
     @Transactional
     public void updateResource(FileResource resource) {
         entityManager.merge(resource);

diff --git a/server/src/main/java/org/eclipse/openvsx/migration/SetContentTypeJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/migration/SetContentTypeJobRequestHandler.java
@@ -0,0 +1,55 @@
+/** ******************************************************************************
+ * Copyright (c) 2022 Precies. Software Ltd and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ * ****************************************************************************** */
+package org.eclipse.openvsx.migration;
+
+import org.eclipse.openvsx.ExtensionProcessor;
+import org.eclipse.openvsx.entities.ExtensionVersion;
+import org.eclipse.openvsx.entities.FileResource;
+import org.jobrunr.jobs.annotations.Job;
+import org.jobrunr.jobs.context.JobRunrDashboardLogger;
+import org.jobrunr.jobs.lambdas.JobRequestHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import java.util.function.Consumer;
+
+@Component
+public class SetContentTypeJobRequestHandler implements JobRequestHandler<MigrationJobRequest> {
+
+    protected final Logger logger = new JobRunrDashboardLogger(LoggerFactory.getLogger(ExtractResourcesJobRequestHandler.class));
+
+    @Autowired
+    MigrationService migrations;
+
+    @Autowired
+    SetContentTypeJobService service;
+
+    @Override
+    @Job(name = "Set content type for published file resources", retries = 3)
+    public void run(MigrationJobRequest jobRequest) throws Exception {
+        var extVersion = migrations.find(jobRequest, ExtensionVersion.class);
+        logger.info("Set content type for: {}.{}-{}@{}", extVersion.getExtension().getNamespace().getName(), extVersion.getExtension().getName(), extVersion.getVersion(), extVersion.getTargetPlatform());
+
+        var entry = migrations.getDownload(extVersion);
+        var extensionFile = migrations.getExtensionFile(entry);
+        try (var processor = new ExtensionProcessor(extensionFile)) {
+            Consumer<FileResource> consumer = resource -> {
+                service.setContentType(extVersion, resource);
+                migrations.deleteResource(resource);
+                migrations.uploadResource(resource);
+            };
+
+            processor.getFileResources(extVersion).forEach(consumer);
+            processor.processEachResource(extVersion, consumer);
+        }
+    }
+}