Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web extension resources return incorrect MIME type #470

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 53 additions & 15 deletions server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,22 @@
import com.fasterxml.jackson.databind.node.MissingNode;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.lang3.StringUtils;

import org.eclipse.openvsx.adapter.ExtensionQueryResult;
import org.eclipse.openvsx.entities.ExtensionVersion;
import org.eclipse.openvsx.entities.FileResource;
import org.eclipse.openvsx.util.*;
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;
import java.io.ByteArrayInputStream;
import java.io.EOFException;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
Expand Down Expand Up @@ -295,17 +302,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, this::getVsixManifest
);

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 @@ -324,6 +336,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 All @@ -340,6 +353,7 @@ public FileResource getBinary(ExtensionVersion extVersion, String binaryName) {
binary.setName(binaryName);
binary.setType(FileResource.DOWNLOAD);
binary.setContent(null);
binary.setContentType("application/zip");
return binary;
}

Expand All @@ -360,6 +374,7 @@ public FileResource generateSha256Checksum(ExtensionVersion extVersion) {
sha256.setName(NamingUtil.toFileFormat(extVersion, ".sha256"));
sha256.setType(FileResource.DOWNLOAD_SHA256);
sha256.setContent(hash.getBytes(StandardCharsets.UTF_8));
sha256.setContentType(MediaType.TEXT_PLAIN_VALUE);
return sha256;
}

Expand Down Expand Up @@ -418,9 +433,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 @@ -439,6 +452,38 @@ public FileResource getLicense(ExtensionVersion extVersion) {
return license;
}

private Map<String, String> loadContentTypes() {
var content = ArchiveUtil.readEntry(zipFile, "[Content_Types].xml");
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.substring(1);
}

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 (StringUtils.isEmpty(extVersion.getLicense())) {
var detection = new LicenseDetection();
Expand All @@ -464,9 +509,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 Down Expand Up @@ -506,12 +549,7 @@ protected FileResource getIcon(ExtensionVersion extVersion) {

var icon = new FileResource();
icon.setExtension(extVersion);
var fileNameIndex = iconPath.lastIndexOf('/');
var iconName = fileNameIndex >= 0
? iconPath.substring(fileNameIndex + 1)
: iconPath;

icon.setName(iconName);
icon.setName(FilenameUtils.getName(iconPath));
icon.setType(FileResource.ICON);
icon.setContent(bytes);
return icon;
Expand Down
22 changes: 0 additions & 22 deletions server/src/main/java/org/eclipse/openvsx/ExtensionValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,13 @@
package org.eclipse.openvsx;

import org.apache.commons.lang3.StringUtils;
import org.apache.tika.Tika;
import org.apache.tika.mime.MediaType;
import org.apache.tika.mime.MimeTypeException;
import org.apache.tika.mime.MimeTypes;
import org.eclipse.openvsx.entities.ExtensionVersion;
import org.eclipse.openvsx.entities.SemanticVersion;
import org.eclipse.openvsx.json.NamespaceDetailsJson;
import org.eclipse.openvsx.util.TargetPlatform;
import org.eclipse.openvsx.util.VersionAlias;
import org.springframework.stereotype.Component;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
Expand Down Expand Up @@ -82,22 +76,6 @@ public List<Issue> validateNamespaceDetails(NamespaceDetailsJson json) {
issues.add(new Issue("Invalid Twitter URL"));
}

if(json.logoBytes != null) {
try (var in = new ByteArrayInputStream(json.logoBytes)) {
var tika = new Tika();
var detectedType = tika.detect(in, json.logo);
var logoType = MimeTypes.getDefaultMimeTypes().getRegisteredMimeType(detectedType);
if(logoType != null) {
json.logo = "logo-" + json.name + "-" + System.currentTimeMillis() + logoType.getExtension();
if(!logoType.getType().equals(MediaType.image("png")) && !logoType.getType().equals(MediaType.image("jpg"))) {
issues.add(new Issue("Namespace logo should be of png or jpg type"));
}
}
} catch (IOException | MimeTypeException e) {
issues.add(new Issue("Failed to read namespace logo"));
}
}

return issues;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
package org.eclipse.openvsx;

import com.google.common.collect.Maps;
import jakarta.persistence.EntityManager;
import jakarta.transaction.Transactional;
import org.apache.commons.lang3.StringUtils;
import org.eclipse.openvsx.cache.CacheService;
import org.eclipse.openvsx.eclipse.EclipseService;
Expand All @@ -28,16 +30,13 @@
import org.springframework.cache.annotation.Cacheable;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.elasticsearch.core.SearchHit;
import org.springframework.data.elasticsearch.core.SearchHits;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.retry.annotation.Retryable;
import org.springframework.stereotype.Component;
import org.springframework.web.server.ResponseStatusException;

import jakarta.persistence.EntityManager;
import jakarta.transaction.Transactional;
import java.io.InputStream;
import java.util.*;
import java.util.stream.Collectors;
Expand Down
23 changes: 23 additions & 0 deletions server/src/main/java/org/eclipse/openvsx/UserService.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@

import com.google.common.base.Joiner;
import org.apache.commons.io.IOUtils;
import org.apache.tika.Tika;
import org.apache.tika.mime.MediaType;
import org.apache.tika.mime.MimeTypeException;
import org.apache.tika.mime.MimeTypes;
import org.eclipse.openvsx.cache.CacheService;
import org.eclipse.openvsx.entities.*;
import org.eclipse.openvsx.json.AccessTokenJson;
Expand Down Expand Up @@ -221,6 +225,23 @@ public ResultJson updateNamespaceDetails(NamespaceDetailsJson details) {
throw new ErrorResultException(message);
}

String contentType = null;
if(details.logoBytes != null) {
try (var in = new ByteArrayInputStream(details.logoBytes)) {
var tika = new Tika();
contentType = tika.detect(in, details.logo);
var logoType = MimeTypes.getDefaultMimeTypes().getRegisteredMimeType(contentType);
if(logoType != null) {
details.logo = "logo-" + details.name + "-" + System.currentTimeMillis() + logoType.getExtension();
if(!logoType.getType().equals(MediaType.image("png")) && !logoType.getType().equals(MediaType.image("jpg"))) {
throw new ErrorResultException("Namespace logo should be of png or jpg type");
}
}
} catch (IOException | MimeTypeException e) {
throw new ErrorResultException("Failed to read namespace logo");
}
}

if(!Objects.equals(details.displayName, namespace.getDisplayName())) {
namespace.setDisplayName(details.displayName);
}
Expand Down Expand Up @@ -259,11 +280,13 @@ public ResultJson updateNamespaceDetails(NamespaceDetailsJson details) {

namespace.setLogoName(details.logo);
namespace.setLogoBytes(details.logoBytes);
namespace.setLogoContentType(contentType);
storeNamespaceLogo(namespace);
} else if (namespace.getLogoStorageType() != null) {
storageUtil.removeNamespaceLogo(namespace);
namespace.setLogoName(null);
namespace.setLogoBytes(null);
namespace.setLogoContentType(null);
namespace.setLogoStorageType(null);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,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
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public class FileResource {
@Basic(fetch = FetchType.LAZY)
byte[] content;

String contentType;

@Column(length = 32)
String storageType;

Expand Down Expand Up @@ -90,6 +92,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
13 changes: 12 additions & 1 deletion server/src/main/java/org/eclipse/openvsx/entities/Namespace.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public class Namespace implements Serializable {
@Column(length = 32)
String logoStorageType;

String logoContentType;

@ElementCollection
@MapKeyColumn(name = "provider")
@Column(name = "social_link")
Expand Down Expand Up @@ -133,6 +135,14 @@ public void setLogoStorageType(String logoStorageType) {
this.logoStorageType = logoStorageType;
}

public String getLogoContentType() {
return logoContentType;
}

public void setLogoContentType(String logoContentType) {
this.logoContentType = logoContentType;
}

public String getWebsite() {
return website;
}
Expand Down Expand Up @@ -206,6 +216,7 @@ public boolean equals(Object o) {
&& Objects.equals(logoName, namespace.logoName)
&& Arrays.equals(logoBytes, namespace.logoBytes)
&& Objects.equals(logoStorageType, namespace.logoStorageType)
&& Objects.equals(logoContentType, namespace.logoContentType)
&& Objects.equals(socialLinks, namespace.socialLinks)
&& Objects.equals(extensions, namespace.extensions)
&& Objects.equals(memberships, namespace.memberships);
Expand All @@ -214,7 +225,7 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = Objects.hash(id, publicId, name, displayName, description, website, supportLink, logoName,
logoStorageType, socialLinks, extensions, memberships);
logoStorageType, logoContentType, socialLinks, extensions, memberships);
result = 31 * result + Arrays.hashCode(logoBytes);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public void run(HandlerJobRequest<?> jobRequest) throws Exception {
fixTargetPlatformMigration();
generateSha256ChecksumMigration();
extensionVersionSignatureMigration();
setFileResourceContentTypeMigration();
setNamespaceLogoContentTypeMigration();
}

private void extractResourcesMigration() {
Expand Down Expand Up @@ -89,4 +91,16 @@ private void extensionVersionSignatureMigration() {
scheduler.enqueue(new HandlerJobRequest<>(GenerateKeyPairJobRequestHandler.class));
}
}

private void setFileResourceContentTypeMigration() {
var jobName = "SetFileResourceContentTypeMigration";
var handler = SetFileResourceContentTypeJobRequestHandler.class;
repositories.findNotMigratedFileResourceContentTypes().forEach(item -> migrations.enqueueMigration(jobName, handler, item));
}

private void setNamespaceLogoContentTypeMigration() {
var jobName = "SetNamespaceLogoContentTypeMigration";
var handler = SetNamespaceLogoContentTypeJobRequestHandler.class;
repositories.findNotMigratedNamespaceLogoContentTypes().forEach(item -> migrations.enqueueMigration(jobName, handler, item));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
* ****************************************************************************** */
package org.eclipse.openvsx.migration;

import jakarta.persistence.EntityManager;
import jakarta.transaction.Transactional;
import org.eclipse.openvsx.entities.ExtensionVersion;
import org.eclipse.openvsx.entities.FileResource;
import org.eclipse.openvsx.entities.MigrationItem;
Expand All @@ -25,8 +27,6 @@
import org.springframework.stereotype.Component;
import org.springframework.web.client.RestTemplate;

import jakarta.persistence.EntityManager;
import jakarta.transaction.Transactional;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
Expand Down
Loading