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.

Fix loggers
Make migration run on every startup for testing purposes

read inputstream before loading content types

Get existing resource from db

make uploadFile transactional

move Transactional to MigrationService

move transactional

get file

delete file
use content of resource
  • Loading branch information
amvanbaren committed Dec 20, 2022
1 parent f061c72 commit 1ca2479
Show file tree
Hide file tree
Showing 41 changed files with 950 additions and 212 deletions.
58 changes: 55 additions & 3 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 Down Expand Up @@ -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.
Expand Down Expand Up @@ -282,17 +288,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 +322,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 @@ -413,6 +425,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();
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 10 additions & 0 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 @@ -45,6 +45,8 @@ public class FileResource {
@Basic(fetch = FetchType.LAZY)
byte[] content;

String contentType;

@Column(length = 32)
String storageType;

Expand Down Expand Up @@ -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;
}
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);
}
}
29 changes: 0 additions & 29 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,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;

Expand All @@ -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");
Expand All @@ -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);
Expand Down
15 changes: 15 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 @@ -20,6 +20,8 @@

import javax.transaction.Transactional;
import java.nio.charset.StandardCharsets;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.util.UUID;

@Component
Expand All @@ -41,6 +43,8 @@ public void runMigrations(ApplicationStartedEvent event) {
extractResourcesMigration();
setPreReleaseMigration();
renameDownloadsMigration();
resetContentTypeMigration(); // TODO remove when done debugging
setContentTypeMigration();
}

private void extractResourcesMigration() {
Expand All @@ -61,6 +65,17 @@ private void renameDownloadsMigration() {
repositories.findNotMigratedRenamedDownloads().forEach(item -> enqueueJob(jobName, handler, item));
}

// TODO remove when done debugging
private void resetContentTypeMigration() {
repositories.findMigratedContentTypes().forEach(item -> item.setMigrationScheduled(false));
}

private void setContentTypeMigration() {
var jobName = "SetContentTypeMigration" + LocalDateTime.now().toEpochSecond(ZoneOffset.UTC); // TODO remove when done debugging
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
53 changes: 46 additions & 7 deletions server/src/main/java/org/eclipse/openvsx/migration/MigrationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -36,20 +47,38 @@ 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);
}

@Transactional
public byte[] getContent(FileResource download) {
download = entityManager.merge(download);
return download.getStorageType().equals(FileResource.STORAGE_DB) ? download.getContent() : null;
}

@Retryable
public Path getExtensionFile(Map.Entry<FileResource, byte[]> entry) {
public Path getFile(Map.Entry<FileResource, byte[]> entry) {
Path extensionFile;
var resource = entry.getKey();
try {
extensionFile = Files.createTempFile("extension_", ".vsix");
var suffixIndex = resource.getName().lastIndexOf('.');
extensionFile = Files.createTempFile("file_", resource.getName().substring(suffixIndex));
} catch (IOException e) {
throw new RuntimeException("Failed to create extension file", e);
throw new RuntimeException("Failed to create file", e);
}

var content = entry.getValue();
if(content == null) {
var download = entry.getKey();
var storage = getStorage(download);
var uri = storage.getLocation(download);
var storage = getStorage(resource);
var uri = storage.getLocation(resource);
restTemplate.execute(uri, HttpMethod.GET, null, response -> {
try(var out = Files.newOutputStream(extensionFile)) {
response.getBody().transferTo(out);
Expand All @@ -61,19 +90,29 @@ public Path getExtensionFile(Map.Entry<FileResource, byte[]> entry) {
try {
Files.write(extensionFile, content);
} catch (IOException e) {
throw new RuntimeException("Failed to write to extension file", e);
throw new RuntimeException("Failed to write to file", e);
}
}

return extensionFile;
}

public void deleteFile(Path filePath) {
try {
Files.delete(filePath);
} catch (IOException e) {
throw new RuntimeException("Failed to delete file");
}
}

@Retryable
@Transactional
public void uploadResource(FileResource resource) {
if(resource.getStorageType().equals(FileResource.STORAGE_DB)) {
return;
}

resource = entityManager.merge(resource);
var storage = getStorage(resource);
storage.uploadFile(resource);
resource.setContent(null);
Expand Down
Loading

0 comments on commit 1ca2479

Please sign in to comment.