Skip to content

Commit

Permalink
Merge pull request #1054 from amvanbaren/fix-webresources
Browse files Browse the repository at this point in the history 
Check that file exists
  • Loading branch information
@amvanbaren
amvanbaren authored Nov 20, 2024
2 parents 0dfaece + bdfa42f commit 6bdfea5
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 23 deletions.
36 changes: 25 additions & 11 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 @@ -11,6 +11,7 @@

import com.google.common.collect.Lists;
import org.apache.commons.lang3.StringUtils;
import org.eclipse.openvsx.cache.CacheService;
import org.eclipse.openvsx.entities.Extension;
import org.eclipse.openvsx.entities.ExtensionVersion;
import org.eclipse.openvsx.entities.FileResource;
Expand All @@ -19,6 +20,8 @@
import org.eclipse.openvsx.search.SearchUtilService;
import org.eclipse.openvsx.storage.StorageUtilService;
import org.eclipse.openvsx.util.*;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand All @@ -28,6 +31,8 @@

import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.*;
import java.util.stream.Collectors;

Expand All @@ -41,13 +46,15 @@

@Component
public class LocalVSCodeService implements IVSCodeService {
protected final Logger logger = LoggerFactory.getLogger(LocalVSCodeService.class);

private final RepositoryService repositories;
private final VersionService versions;
private final SearchUtilService search;
private final StorageUtilService storageUtil;
private final ExtensionVersionIntegrityService integrityService;
private final WebResourceService webResources;
private final CacheService cache;

private final StreamingResponseBody builtinExtensionResponse = outputStream -> {
var message = "Built-in extension namespace '" + BuiltInExtensionUtil.getBuiltInNamespace() + "' not allowed";
Expand All @@ -63,14 +70,16 @@ public LocalVSCodeService(
SearchUtilService search,
StorageUtilService storageUtil,
ExtensionVersionIntegrityService integrityService,
WebResourceService webResources
WebResourceService webResources,
CacheService cache
) {
this.repositories = repositories;
this.versions = versions;
this.search = search;
this.storageUtil = storageUtil;
this.integrityService = integrityService;
this.webResources = webResources;
this.cache = cache;
}

@Override
Expand Down Expand Up @@ -319,17 +328,26 @@ public ResponseEntity<StreamingResponseBody> getAsset(
return storageUtil.getFileResponse(resource);
} else if(asset.startsWith(FILE_WEB_RESOURCES + "/extension/")) {
var name = asset.substring((FILE_WEB_RESOURCES.length() + 1));
var file = webResources.getWebResource(namespace, extensionName, targetPlatform, version, name, false);
if(file == null) {
throw new NotFoundException();
}

var file = getWebResource(namespace, extensionName, targetPlatform, version, name, false);
return storageUtil.getFileResponse(file);
}

throw new NotFoundException();
}

private Path getWebResource(String namespaceName, String extensionName, String targetPlatform, String version, String name, boolean browse) {
var file = webResources.getWebResource(namespaceName, extensionName, targetPlatform, version, name, browse);
if(file == null) {
throw new NotFoundException();
}
if(!Files.exists(file)) {
logger.error("File doesn't exist {}", file);
cache.evictWebResourceFile(namespaceName, extensionName, null, version, name);
throw new NotFoundException();
}
return file;
}

private String builtinExtensionMessage() {
return "Built-in extension namespace '" + BuiltInExtensionUtil.getBuiltInNamespace() + "' not allowed";
}
Expand Down Expand Up @@ -381,11 +399,7 @@ public ResponseEntity<StreamingResponseBody> browse(String namespaceName, String
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(builtinExtensionResponse);
}

var file = webResources.getWebResource(namespaceName, extensionName, null, version, path, true);
if(file == null) {
throw new NotFoundException();
}

var file = getWebResource(namespaceName, extensionName, null, version, path, true);
return storageUtil.getFileResponse(file);
}

Expand Down
14 changes: 13 additions & 1 deletion server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@
package org.eclipse.openvsx.adapter;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.eclipse.openvsx.cache.CacheService;
import org.eclipse.openvsx.entities.FileResource;
import org.eclipse.openvsx.repositories.RepositoryService;
import org.eclipse.openvsx.storage.StorageUtilService;
import org.eclipse.openvsx.util.ErrorResultException;
import org.eclipse.openvsx.util.NamingUtil;
import org.eclipse.openvsx.util.UrlUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.stereotype.Component;

Expand All @@ -32,12 +35,16 @@
@Component
public class WebResourceService {

protected final Logger logger = LoggerFactory.getLogger(WebResourceService.class);

private final StorageUtilService storageUtil;
private final RepositoryService repositories;
private final CacheService cache;

public WebResourceService(StorageUtilService storageUtil, RepositoryService repositories) {
public WebResourceService(StorageUtilService storageUtil, RepositoryService repositories, CacheService cache) {
this.storageUtil = storageUtil;
this.repositories = repositories;
this.cache = cache;
}

@Cacheable(value = CACHE_WEB_RESOURCE_FILES, keyGenerator = GENERATOR_FILES)
Expand All @@ -56,6 +63,11 @@ public Path getWebResource(String namespace, String extension, String targetPlat
if(path == null) {
return null;
}
if(!Files.exists(path)) {
logger.error("File doesn't exist {}", path);
cache.evictExtensionFile(download);
return null;
}

try(var zip = new ZipFile(path.toFile())) {
var fileEntry = zip.getEntry(name);
Expand Down
24 changes: 23 additions & 1 deletion server/src/main/java/org/eclipse/openvsx/cache/CacheService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import org.eclipse.openvsx.entities.Extension;
import org.eclipse.openvsx.entities.ExtensionVersion;
import org.eclipse.openvsx.entities.FileResource;
import org.eclipse.openvsx.entities.UserData;
import org.eclipse.openvsx.repositories.RepositoryService;
import org.eclipse.openvsx.util.TargetPlatform;
Expand Down Expand Up @@ -42,17 +43,20 @@ public class CacheService {
private final RepositoryService repositories;
private final ExtensionJsonCacheKeyGenerator extensionJsonCacheKey;
private final LatestExtensionVersionCacheKeyGenerator latestExtensionVersionCacheKey;
private final FilesCacheKeyGenerator filesCacheKeyGenerator;

public CacheService(
CacheManager cacheManager,
RepositoryService repositories,
ExtensionJsonCacheKeyGenerator extensionJsonCacheKey,
LatestExtensionVersionCacheKeyGenerator latestExtensionVersionCacheKey
LatestExtensionVersionCacheKeyGenerator latestExtensionVersionCacheKey,
FilesCacheKeyGenerator filesCacheKeyGenerator
) {
this.cacheManager = cacheManager;
this.repositories = repositories;
this.extensionJsonCacheKey = extensionJsonCacheKey;
this.latestExtensionVersionCacheKey = latestExtensionVersionCacheKey;
this.filesCacheKeyGenerator = filesCacheKeyGenerator;
}

public void evictNamespaceDetails() {
Expand Down Expand Up @@ -154,4 +158,22 @@ private void invalidateCache(String cacheName) {

cache.invalidate();
}

public void evictExtensionFile(FileResource download) {
var cache = cacheManager.getCache(CACHE_EXTENSION_FILES);
if(cache == null) {
return;
}

cache.evict(filesCacheKeyGenerator.generate(download));
}

public void evictWebResourceFile(String namespaceName, String extensionName, String targetPlatform, String version, String path) {
var cache = cacheManager.getCache(CACHE_WEB_RESOURCE_FILES);
if(cache == null) {
return;
}

cache.evict(filesCacheKeyGenerator.generate(namespaceName, extensionName, targetPlatform, version, path));
}
}
15 changes: 9 additions & 6 deletions server/src/main/java/org/eclipse/openvsx/cache/FilesCacheKeyGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,20 @@ public Object generate(Object target, Method method, Object... params) {
return generate(namespace, extension, targetPlatform, version, name);
}
if(target instanceof IStorageService) {
var resource = (FileResource) params[0];
var extVersion = resource.getExtension();
var extension = extVersion.getExtension();
var namespace = extension.getNamespace();
return generate(namespace.getName(), extension.getName(), extVersion.getTargetPlatform(), extVersion.getVersion(), resource.getName());
return generate((FileResource) params[0]);
}

throw new UnsupportedOperationException();
}

private String generate(String namespace, String extension, String targetPlatform, String version, String name) {
public String generate(FileResource resource) {
var extVersion = resource.getExtension();
var extension = extVersion.getExtension();
var namespace = extension.getNamespace();
return generate(namespace.getName(), extension.getName(), extVersion.getTargetPlatform(), extVersion.getVersion(), resource.getName());
}

public String generate(String namespace, String extension, String targetPlatform, String version, String name) {
return UrlUtil.createApiFileUrl("", namespace, extension, targetPlatform, version, name);
}
}
9 changes: 5 additions & 4 deletions server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -933,8 +933,8 @@ TokenService tokenService(
}

@Bean
WebResourceService webResourceService(StorageUtilService storageUtil, RepositoryService repositories) {
return new WebResourceService(storageUtil, repositories);
WebResourceService webResourceService(StorageUtilService storageUtil, RepositoryService repositories, CacheService cache) {
return new WebResourceService(storageUtil, repositories, cache);
}

@Bean
Expand All @@ -944,9 +944,10 @@ LocalVSCodeService localVSCodeService(
SearchUtilService search,
StorageUtilService storageUtil,
ExtensionVersionIntegrityService integrityService,
WebResourceService webResourceService
WebResourceService webResourceService,
CacheService cache
) {
return new LocalVSCodeService(repositories, versions, search, storageUtil, integrityService, webResourceService);
return new LocalVSCodeService(repositories, versions, search, storageUtil, integrityService, webResourceService, cache);
}

@Bean
Expand Down

0 comments on commit 6bdfea5

Please sign in to comment.