Skip to content

Commit

Permalink
Web extension resources return incorrect MIME type
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
amvanbaren committed Dec 20, 2022
1 parent f061c72 commit fe0f247
Show file tree
Hide file tree
Showing 42 changed files with 1,125 additions and 268 deletions.
72 changes: 58 additions & 14 deletions server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -26,6 +27,7 @@
import com.fasterxml.jackson.databind.node.MissingNode;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;

import org.apache.commons.io.FilenameUtils;
import org.eclipse.openvsx.entities.ExtensionVersion;
import org.eclipse.openvsx.entities.FileResource;
import org.eclipse.openvsx.util.ArchiveUtil;
Expand All @@ -36,6 +38,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.
Expand Down Expand Up @@ -282,17 +289,22 @@ private List<String> getEngines(JsonNode node) {
}

public List<FileResource> getFileResources(ExtensionVersion extVersion) {
var resources = new ArrayList<FileResource>();
readInputStream();
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 -> {
Expand All @@ -311,6 +323,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)
Expand Down Expand Up @@ -393,9 +406,7 @@ public FileResource getLicense(ExtensionVersion extVersion) {
var fileName = matcher.group("file");
var bytes = ArchiveUtil.readEntry(zipFile, "extension/" + fileName);
if (bytes != null) {
var lastSegmentIndex = fileName.lastIndexOf('/');
var lastSegment = fileName.substring(lastSegmentIndex + 1);
license.setName(lastSegment);
license.setName(FilenameUtils.getName(fileName));
license.setContent(bytes);
detectLicense(bytes, extVersion);
return license;
Expand All @@ -413,6 +424,44 @@ 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 fileExtension = FilenameUtils.getExtension(resource.getName());
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();
Expand All @@ -425,9 +474,7 @@ private Pair<byte[], String> readFromAlternateNames(String[] names) {
var entry = ArchiveUtil.getEntryIgnoreCase(zipFile, name);
if (entry != null) {
var bytes = ArchiveUtil.readEntry(zipFile, entry);
var lastSegmentIndex = entry.getName().lastIndexOf('/');
var lastSegment = entry.getName().substring(lastSegmentIndex + 1);
return Pair.of(bytes, lastSegment);
return Pair.of(bytes, FilenameUtils.getName(entry.getName()));
}
}
return null;
Expand All @@ -442,13 +489,10 @@ protected FileResource getIcon(ExtensionVersion extVersion) {
var bytes = ArchiveUtil.readEntry(zipFile, "extension/" + iconPathStr);
if (bytes == null)
return null;

var icon = new FileResource();
icon.setExtension(extVersion);
var fileNameIndex = iconPathStr.lastIndexOf('/');
if (fileNameIndex >= 0)
icon.setName(iconPathStr.substring(fileNameIndex + 1));
else
icon.setName(iconPathStr);
icon.setName(FilenameUtils.getName(iconPathStr));
icon.setType(FileResource.ICON);
icon.setContent(bytes);
return icon;
Expand Down
4 changes: 1 addition & 3 deletions server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import javax.transaction.Transactional;

import static org.eclipse.openvsx.adapter.ExtensionQueryParam.Criterion.*;
import static org.eclipse.openvsx.adapter.ExtensionQueryParam.*;
import static org.eclipse.openvsx.adapter.ExtensionQueryResult.Extension.FLAG_PREVIEW;
Expand Down Expand Up @@ -404,7 +402,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();
Expand Down
12 changes: 10 additions & 2 deletions server/src/main/java/org/eclipse/openvsx/entities/FileResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
********************************************************************************/
package org.eclipse.openvsx.entities;

import org.hibernate.annotations.Fetch;

import javax.persistence.*;

@Entity
Expand Down Expand Up @@ -45,6 +43,8 @@ public class FileResource {
@Basic(fetch = FetchType.LAZY)
byte[] content;

String contentType;

@Column(length = 32)
String storageType;

Expand Down Expand Up @@ -88,6 +88,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;
}
Expand Down
11 changes: 6 additions & 5 deletions server/src/main/java/org/eclipse/openvsx/migration/ExtractResourcesJobRequestHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
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;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -21,7 +21,7 @@
@Component
public class ExtractResourcesJobRequestHandler implements JobRequestHandler<MigrationJobRequest> {

protected final Logger logger = new JobRunrDashboardLogger(LoggerFactory.getLogger(ExtractResourcesJobRequestHandler.class));
protected final Logger logger = LoggerFactory.getLogger(ExtractResourcesJobRequestHandler.class);

@Autowired
ExtractResourcesJobService service;
Expand All @@ -32,12 +32,12 @@ 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 extensionFile = migrations.getExtensionFile(entry);
var entry = migrations.getDownload(extVersion);
var extensionFile = migrations.getFile(entry);
var download = entry.getKey();
try(var extProcessor = new ExtensionProcessor(extensionFile)) {
extProcessor.processEachResource(download.getExtension(), (resource) -> {
Expand All @@ -48,5 +48,6 @@ public void run(MigrationJobRequest jobRequest) throws Exception {
}

service.deleteWebResources(extVersion);
migrations.deleteFile(extensionFile);
}
}
31 changes: 0 additions & 31 deletions server/src/main/java/org/eclipse/openvsx/migration/ExtractResourcesJobService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,45 +12,21 @@
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;

@Component
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");
Expand All @@ -61,13 +37,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);
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/eclipse/openvsx/migration/MigrationRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public void runMigrations(ApplicationStartedEvent event) {
extractResourcesMigration();
setPreReleaseMigration();
renameDownloadsMigration();
setContentTypeMigration();
}

private void extractResourcesMigration() {
Expand All @@ -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));
Expand Down
Loading

0 comments on commit fe0f247

Please sign in to comment.