From 30324b1d469925a10083f9133afb5ceaed38b4c2 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Sat, 8 Aug 2026 12:59:44 +0200 Subject: [PATCH 1/4] fix: limit decompressed size when serving vscode/unpkg web resources WebResourceService.writeBinaryFile extracted a VSIX zip entry to disk with no bound on the decompressed size. A small, highly compressed VSIX could therefore expand into an oversized file under java.io.tmpdir when a single entry was requested via /vscode/unpkg/, and the upload-time size check only limits the compressed package. - Reject entries whose declared size exceeds a new ovsx.caching.files-webresource.max-file-size limit (default 32 MB, matching ArchiveUtil.MAX_ENTRY_SIZE) before extracting, and bound the actual copy with SizeLimitInputStream so a zip entry with an inconsistent header can't produce more bytes than declared. - FileUtil.writeSync now deletes a partially written file if the writer fails, instead of leaving it behind. Previously a failed write (e.g. disk full) left a truncated file that permanently blocked retries at that cache path, since writeSync only (re-)invokes the writer when the file doesn't already exist. - Switch the web resource cache from counting entries (maximumSize) to weighing them by file size (maximumWeight + new FileSizeWeigher), configurable via ovsx.caching.files-webresource.max-total-size (default 2 GiB), so the cache is bounded by total disk usage rather than file count. Co-Authored-By: Claude Sonnet 5 --- .../openvsx/adapter/WebResourceService.java | 30 ++++- .../eclipse/openvsx/cache/CacheConfig.java | 7 +- .../openvsx/cache/FileSizeWeigher.java | 46 +++++++ .../org/eclipse/openvsx/util/FileUtil.java | 26 +++- .../openvsx/adapter/VSCodeAPITest.java | 2 +- .../adapter/WebResourceServiceTest.java | 117 ++++++++++++++++++ .../openvsx/cache/FileSizeWeigherTest.java | 46 +++++++ .../eclipse/openvsx/util/FileUtilTest.java | 78 ++++++++++++ 8 files changed, 344 insertions(+), 8 deletions(-) create mode 100644 server/src/main/java/org/eclipse/openvsx/cache/FileSizeWeigher.java create mode 100644 server/src/test/java/org/eclipse/openvsx/adapter/WebResourceServiceTest.java create mode 100644 server/src/test/java/org/eclipse/openvsx/cache/FileSizeWeigherTest.java create mode 100644 server/src/test/java/org/eclipse/openvsx/util/FileUtilTest.java diff --git a/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java b/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java index a53ed6642..29a88f402 100644 --- a/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java +++ b/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java @@ -18,8 +18,10 @@ import java.util.zip.ZipFile; import io.micrometer.observation.annotation.Observed; +import org.apache.commons.io.FileUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; import org.springframework.cache.annotation.Cacheable; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Service; @@ -34,6 +36,7 @@ import org.eclipse.openvsx.util.ErrorResultException; import org.eclipse.openvsx.util.FileUtil; import org.eclipse.openvsx.util.NamingUtil; +import org.eclipse.openvsx.util.SizeLimitInputStream; import org.eclipse.openvsx.util.UrlUtil; import static org.eclipse.openvsx.cache.CacheService.*; @@ -48,17 +51,24 @@ public class WebResourceService { private final FilesCacheKeyGenerator filesCacheKeyGenerator; private final JsonMapper jsonMapper; + // Limit the decompressed size of a single served web resource, default 32 MB matching + // ArchiveUtil.MAX_ENTRY_SIZE. Without this, a small VSIX containing a highly compressed entry + // could exhaust the java.io.tmpdir filesystem when that entry is extracted on request. + private final long maxFileSize; + public WebResourceService( StorageUtilService storageUtil, RepositoryService repositories, CacheService cache, - FilesCacheKeyGenerator filesCacheKeyGenerator + FilesCacheKeyGenerator filesCacheKeyGenerator, + @Value("${ovsx.caching.files-webresource.max-file-size:33554432}") long maxFileSize ) { this.storageUtil = storageUtil; this.repositories = repositories; this.cache = cache; this.filesCacheKeyGenerator = filesCacheKeyGenerator; this.jsonMapper = JsonMapper.shared(); + this.maxFileSize = maxFileSize; } public Path getExtensionDownload(String namespace, String extension, String targetPlatform, String version) { @@ -154,9 +164,23 @@ private String getFileExtension(ZipEntry fileEntry) { } private void writeBinaryFile(Path file, ZipFile zip, ZipEntry fileEntry) { + var declaredSize = fileEntry.getSize(); + if (declaredSize < 0) { + throw new ErrorResultException("The file " + fileEntry.getName() + " has an unknown size."); + } + if (declaredSize > maxFileSize) { + var maxSize = FileUtils.byteCountToDisplaySize(maxFileSize); + throw new ErrorResultException( + "The file " + fileEntry.getName() + " exceeds the size limit of " + maxSize + ".", + HttpStatus.CONTENT_TOO_LARGE); + } + FileUtil.writeSync(file, p -> { - try (var in = zip.getInputStream(fileEntry)) { - Files.copy(in, p); + // Wrap in SizeLimitInputStream bounded to the declared size: a zip entry can lie about + // its uncompressed size, so this stops the copy the moment more bytes than declared + // have been read instead of trusting the header. + try (var in = zip.getInputStream(fileEntry); var limited = new SizeLimitInputStream(in, declaredSize)) { + Files.copy(limited, p); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/server/src/main/java/org/eclipse/openvsx/cache/CacheConfig.java b/server/src/main/java/org/eclipse/openvsx/cache/CacheConfig.java index edafe3b0c..9765bae38 100644 --- a/server/src/main/java/org/eclipse/openvsx/cache/CacheConfig.java +++ b/server/src/main/java/org/eclipse/openvsx/cache/CacheConfig.java @@ -75,12 +75,15 @@ public Cache extensionCache( @Bean public Cache webResourceCache( @Value("${ovsx.caching.files-webresource.tti:PT1H}") Duration timeToIdle, - @Value("${ovsx.caching.files-webresource.max-size:150}") long maxSize + // 2 GiB total; each entry's weight is its file size in bytes rather than a flat 1, so + // this bounds the cache's disk footprint instead of just the number of cached files. + @Value("${ovsx.caching.files-webresource.max-total-size:2147483648}") long maxTotalSize ) { return Caffeine.newBuilder() .removalListener(new ExpiredFileListener()) .expireAfterAccess(timeToIdle) - .maximumSize(maxSize) + .maximumWeight(maxTotalSize) + .weigher(new FileSizeWeigher()) .scheduler(Scheduler.systemScheduler()) .recordStats() .build(); diff --git a/server/src/main/java/org/eclipse/openvsx/cache/FileSizeWeigher.java b/server/src/main/java/org/eclipse/openvsx/cache/FileSizeWeigher.java new file mode 100644 index 000000000..3af838b30 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/cache/FileSizeWeigher.java @@ -0,0 +1,46 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ +package org.eclipse.openvsx.cache; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import com.github.benmanes.caffeine.cache.Weigher; +import org.jspecify.annotations.NonNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Weighs a cached {@link Path} by its file size in bytes, so a {@code maximumWeight} cache bounds + * its total disk footprint instead of just the number of cached files. + */ +public class FileSizeWeigher implements Weigher { + private static final Logger logger = LoggerFactory.getLogger(FileSizeWeigher.class); + + @Override + public int weigh(@NonNull Object key, @NonNull Object value) { + if (!(value instanceof Path path)) { + return 1; + } + + try { + return (int) Math.min(Files.size(path), Integer.MAX_VALUE); + } catch (IOException e) { + // Can't determine the size, e.g. the file was already deleted; weigh it heavily so + // it doesn't linger and displace entries whose size is known. + logger.warn("Failed to determine size of cached file {}", path, e); + return Integer.MAX_VALUE; + } + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/util/FileUtil.java b/server/src/main/java/org/eclipse/openvsx/util/FileUtil.java index 1b70d9ac0..190228446 100644 --- a/server/src/main/java/org/eclipse/openvsx/util/FileUtil.java +++ b/server/src/main/java/org/eclipse/openvsx/util/FileUtil.java @@ -9,6 +9,7 @@ * ****************************************************************************** */ package org.eclipse.openvsx.util; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; @@ -16,8 +17,13 @@ import java.util.Map; import java.util.function.Consumer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class FileUtil { + private static final Logger logger = LoggerFactory.getLogger(FileUtil.class); + private static final Map LOCKS; static { @@ -34,7 +40,10 @@ private FileUtil() { } /*** - * Write to file synchronously, if it doesn't already exist. + * Write to file synchronously, if it doesn't already exist. If the writer fails partway + * through, the partial file is deleted so a later call doesn't mistake it for a completed + * write (writeSync only (re)invokes the writer when the path doesn't yet exist) and so it + * doesn't linger on disk. * @param path File path to write to * @param writer Writes to file */ @@ -45,8 +54,21 @@ public static void writeSync(Path path, Consumer writer) { } synchronized (lock) { if (!Files.exists(path)) { - writer.accept(path); + try { + writer.accept(path); + } catch (RuntimeException e) { + deleteQuietly(path); + throw e; + } } } } + + private static void deleteQuietly(Path path) { + try { + Files.deleteIfExists(path); + } catch (IOException e) { + logger.warn("Failed to delete partial file {}", path, e); + } + } } diff --git a/server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java b/server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java index d4ddc3b2d..3952aefb8 100644 --- a/server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java @@ -1376,7 +1376,7 @@ WebResourceService webResourceService( CacheService cache, FilesCacheKeyGenerator filesCacheKeyGenerator ) { - return new WebResourceService(storageUtil, repositories, cache, filesCacheKeyGenerator); + return new WebResourceService(storageUtil, repositories, cache, filesCacheKeyGenerator, 33_554_432L); } @Bean diff --git a/server/src/test/java/org/eclipse/openvsx/adapter/WebResourceServiceTest.java b/server/src/test/java/org/eclipse/openvsx/adapter/WebResourceServiceTest.java new file mode 100644 index 000000000..17f9eba2c --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/adapter/WebResourceServiceTest.java @@ -0,0 +1,117 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.adapter; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.junit.jupiter.api.Test; + +import org.eclipse.openvsx.cache.CacheService; +import org.eclipse.openvsx.cache.FilesCacheKeyGenerator; +import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.storage.StorageUtilService; +import org.eclipse.openvsx.util.ErrorResultException; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.springframework.http.HttpStatus.CONTENT_TOO_LARGE; +import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; + +class WebResourceServiceTest { + + private final FilesCacheKeyGenerator filesCacheKeyGenerator = new FilesCacheKeyGenerator(); + + @Test + void testRejectsEntryDeclaredLargerThanLimit() throws Exception { + var extensionDownloadPath = resourcePath("wrong-size.zip"); + var service = newService(8192L); + + assertThatThrownBy( + () -> service.getWebResource( + "ns", + "ext", + null, + "1.0.0", + "extension/README.md", + extensionDownloadPath)) + .isExactlyInstanceOf(ErrorResultException.class) + .hasMessage("The file extension/README.md exceeds the size limit of 8 KB.") + .satisfies(e -> assertThat(((ErrorResultException) e).getStatus()).isEqualTo(CONTENT_TOO_LARGE)); + + // rejected before any extraction: nothing should have been written to the file cache + var cachedPath = filesCacheKeyGenerator + .generateCachedWebResourcePath("ns", "ext", null, "1.0.0", "extension/README.md", ".md"); + assertThat(Files.exists(cachedPath)).isFalse(); + } + + @Test + void testStopsCopyWhenActualBytesExceedDeclaredSize() throws Exception { + // extension/package.json declares a size of 0 but its compressed payload decompresses to + // more than 0 bytes, simulating a zip entry whose header lies about the decompressed size + var extensionDownloadPath = resourcePath("wrong-size.zip"); + var service = newService(33_554_432L); + + assertThatThrownBy( + () -> service.getWebResource( + "ns", + "ext", + null, + "1.0.0", + "extension/package.json", + extensionDownloadPath)) + .isExactlyInstanceOf(ErrorResultException.class) + .hasMessageContaining("File size exceeds limit of 0 bytes") + .satisfies(e -> assertThat(((ErrorResultException) e).getStatus()).isEqualTo(INTERNAL_SERVER_ERROR)); + + // the partial file written before the limit tripped must not be left behind + var cachedPath = filesCacheKeyGenerator + .generateCachedWebResourcePath("ns", "ext", null, "1.0.0", "extension/package.json", ".json"); + assertThat(Files.exists(cachedPath)).isFalse(); + } + + @Test + void testServesFileWithinLimit() throws Exception { + var extensionDownloadPath = resourcePath("todo-tree.zip"); + var service = newService(33_554_432L); + + var cachedPath = service.getWebResource( + "ns", + "ext", + null, + "1.0.0", + "extension/package.json", + extensionDownloadPath); + + assertThat(cachedPath).isNotNull(); + assertThat(Files.size(cachedPath)).isEqualTo(44712); + Files.deleteIfExists(cachedPath); + } + + private WebResourceService newService(long maxFileSize) { + return new WebResourceService( + mock(StorageUtilService.class), + mock(RepositoryService.class), + mock(CacheService.class), + filesCacheKeyGenerator, + maxFileSize); + } + + private Path resourcePath(String name) throws Exception { + var url = getClass().getResource("/org/eclipse/openvsx/util/" + name); + assertThat(url).isNotNull(); + return Paths.get(url.toURI()); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/cache/FileSizeWeigherTest.java b/server/src/test/java/org/eclipse/openvsx/cache/FileSizeWeigherTest.java new file mode 100644 index 000000000..b5c3f93d8 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/cache/FileSizeWeigherTest.java @@ -0,0 +1,46 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.cache; + +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.Assertions.assertThat; + +class FileSizeWeigherTest { + + private final FileSizeWeigher weigher = new FileSizeWeigher(); + + @Test + void testWeighsByFileSize(@TempDir Path tmpDir) throws Exception { + var path = tmpDir.resolve("resource.bin"); + Files.write(path, new byte[1234]); + + assertThat(weigher.weigh("key", path)).isEqualTo(1234); + } + + @Test + void testWeighsMissingFileAsMax() { + var path = Path.of("/does/not/exist"); + + assertThat(weigher.weigh("key", path)).isEqualTo(Integer.MAX_VALUE); + } + + @Test + void testWeighsNonPathValueAsOne() { + assertThat(weigher.weigh("key", "not-a-path")).isEqualTo(1); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/util/FileUtilTest.java b/server/src/test/java/org/eclipse/openvsx/util/FileUtilTest.java new file mode 100644 index 000000000..4a7fd6520 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/util/FileUtilTest.java @@ -0,0 +1,78 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.util; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class FileUtilTest { + + @Test + void testDeletesPartialFileWhenWriterFails(@TempDir Path tmpDir) { + var path = tmpDir.resolve("partial.tmp"); + + assertThatThrownBy(() -> FileUtil.writeSync(path, p -> { + try { + Files.writeString(p, "partial content"); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + throw new RuntimeException("write failed partway through"); + })).hasMessage("write failed partway through"); + + assertThat(Files.exists(path)).isFalse(); + } + + @Test + void testRetriesAfterAFailedWrite(@TempDir Path tmpDir) { + var path = tmpDir.resolve("retry.tmp"); + + assertThatThrownBy(() -> FileUtil.writeSync(path, p -> { + throw new RuntimeException("first attempt fails"); + })).hasMessage("first attempt fails"); + assertThat(Files.exists(path)).isFalse(); + + // a later call for the same path must not see a leftover partial file and skip writing + FileUtil.writeSync(path, p -> { + try { + Files.writeString(p, "content"); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + + assertThat(Files.exists(path)).isTrue(); + } + + @Test + void testDoesNotOverwriteAnExistingFile() throws IOException { + var path = Files.createTempFile("openvsx-file-util-test", ".tmp"); + try { + Files.writeString(path, "original"); + FileUtil.writeSync(path, p -> { + throw new AssertionError("writer must not run when the file already exists"); + }); + assertThat(Files.readString(path)).isEqualTo("original"); + } finally { + Files.deleteIfExists(path); + } + } +} From 452f7052cefb3a1baad6b802399c4c1ac3853843 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Mon, 24 Aug 2026 13:51:57 +0200 Subject: [PATCH 2/4] fix: bound webResourceCache by both entry count and total byte size Caffeine can't combine maximumSize and maximumWeight on one cache, but the same effect is achievable with a single maximumWeight cache by giving every entry a floor weight: FileSizeWeigher now charges each cached file at least ceil(maxTotalSize / maxEntries) of the weight budget, even if its real size is smaller. This restores the previously-removed ovsx.caching.files-webresource.max-size (entry count, default 150) alongside max-total-size (bytes, default 2 GiB) introduced in this PR, so the cache respects both limits at once: an entry larger than its floor share is still weighed by its real size (byte budget holds), while every entry claims at least a 1/maxEntries share of the budget (entry count can never exceed maxEntries). --- .../eclipse/openvsx/cache/CacheConfig.java | 8 +++++- .../openvsx/cache/FileSizeWeigher.java | 20 ++++++++++++++- .../openvsx/cache/FileSizeWeigherTest.java | 25 ++++++++++++++++++- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/cache/CacheConfig.java b/server/src/main/java/org/eclipse/openvsx/cache/CacheConfig.java index 9765bae38..2ee919e5b 100644 --- a/server/src/main/java/org/eclipse/openvsx/cache/CacheConfig.java +++ b/server/src/main/java/org/eclipse/openvsx/cache/CacheConfig.java @@ -75,15 +75,21 @@ public Cache extensionCache( @Bean public Cache webResourceCache( @Value("${ovsx.caching.files-webresource.tti:PT1H}") Duration timeToIdle, + @Value("${ovsx.caching.files-webresource.max-size:150}") long maxEntries, // 2 GiB total; each entry's weight is its file size in bytes rather than a flat 1, so // this bounds the cache's disk footprint instead of just the number of cached files. @Value("${ovsx.caching.files-webresource.max-total-size:2147483648}") long maxTotalSize ) { + // Charge every entry at least this much of the weight budget, so at most maxEntries entries + // can ever fit regardless of how small they are -- the same cache then respects both limits: + // total bytes stay within maxTotalSize (a file's real size still counts when it's the larger + // of the two), and the entry count stays within maxEntries (its floor share of the budget). + var floorWeight = maxEntries > 0 ? Math.ceilDiv(maxTotalSize, maxEntries) : 0; return Caffeine.newBuilder() .removalListener(new ExpiredFileListener()) .expireAfterAccess(timeToIdle) .maximumWeight(maxTotalSize) - .weigher(new FileSizeWeigher()) + .weigher(new FileSizeWeigher(floorWeight)) .scheduler(Scheduler.systemScheduler()) .recordStats() .build(); diff --git a/server/src/main/java/org/eclipse/openvsx/cache/FileSizeWeigher.java b/server/src/main/java/org/eclipse/openvsx/cache/FileSizeWeigher.java index 3af838b30..5cbe23e53 100644 --- a/server/src/main/java/org/eclipse/openvsx/cache/FileSizeWeigher.java +++ b/server/src/main/java/org/eclipse/openvsx/cache/FileSizeWeigher.java @@ -24,10 +24,27 @@ /** * Weighs a cached {@link Path} by its file size in bytes, so a {@code maximumWeight} cache bounds * its total disk footprint instead of just the number of cached files. + *

+ * Every entry is floored to at least {@code floorWeight} of the weight budget, even if its actual + * file is smaller. Combined with a cache's {@code maximumWeight(maxTotalSize)}, setting + * {@code floorWeight = ceil(maxTotalSize / maxEntries)} makes the same cache respect both a total + * byte budget and a maximum entry count at once: no more than {@code maxEntries} entries can ever + * fit, since each one claims at least a {@code 1/maxEntries} share of the budget, while entries + * larger than that share are still weighed by their real size so the byte budget itself holds. */ public class FileSizeWeigher implements Weigher { private static final Logger logger = LoggerFactory.getLogger(FileSizeWeigher.class); + private final long floorWeight; + + /** + * @param floorWeight the minimum weight charged to every entry, regardless of its actual file + * size; pass {@code 0} to weigh purely by actual file size. + */ + public FileSizeWeigher(long floorWeight) { + this.floorWeight = floorWeight; + } + @Override public int weigh(@NonNull Object key, @NonNull Object value) { if (!(value instanceof Path path)) { @@ -35,7 +52,8 @@ public int weigh(@NonNull Object key, @NonNull Object value) { } try { - return (int) Math.min(Files.size(path), Integer.MAX_VALUE); + var size = Math.max(Files.size(path), floorWeight); + return (int) Math.min(size, Integer.MAX_VALUE); } catch (IOException e) { // Can't determine the size, e.g. the file was already deleted; weigh it heavily so // it doesn't linger and displace entries whose size is known. diff --git a/server/src/test/java/org/eclipse/openvsx/cache/FileSizeWeigherTest.java b/server/src/test/java/org/eclipse/openvsx/cache/FileSizeWeigherTest.java index b5c3f93d8..47a7dd2fb 100644 --- a/server/src/test/java/org/eclipse/openvsx/cache/FileSizeWeigherTest.java +++ b/server/src/test/java/org/eclipse/openvsx/cache/FileSizeWeigherTest.java @@ -22,7 +22,7 @@ class FileSizeWeigherTest { - private final FileSizeWeigher weigher = new FileSizeWeigher(); + private final FileSizeWeigher weigher = new FileSizeWeigher(0); @Test void testWeighsByFileSize(@TempDir Path tmpDir) throws Exception { @@ -43,4 +43,27 @@ void testWeighsMissingFileAsMax() { void testWeighsNonPathValueAsOne() { assertThat(weigher.weigh("key", "not-a-path")).isEqualTo(1); } + + @Test + void testFloorsSmallFilesToTheFloorWeight(@TempDir Path tmpDir) throws Exception { + // Simulates a cache sized for at most 10 entries out of a 10_000 byte budget: a file far + // smaller than its 1000-byte share is still charged the full share, so 10 of them exhaust + // the budget and a maximumWeight(10_000) cache can never hold more than 10 entries. + var path = tmpDir.resolve("tiny.bin"); + Files.write(path, new byte[10]); + var weigher = new FileSizeWeigher(1000); + + assertThat(weigher.weigh("key", path)).isEqualTo(1000); + } + + @Test + void testStillWeighsByActualSizeWhenAboveTheFloor(@TempDir Path tmpDir) throws Exception { + // A file bigger than its floor share is charged its real size, so the byte budget is still + // the binding constraint for entries that are individually large. + var path = tmpDir.resolve("large.bin"); + Files.write(path, new byte[5000]); + var weigher = new FileSizeWeigher(1000); + + assertThat(weigher.weigh("key", path)).isEqualTo(5000); + } } From 2127cbd9cc011b366e26378ec3fa1b0ec217e3af Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Mon, 24 Aug 2026 14:01:06 +0200 Subject: [PATCH 3/4] fix: disable web resource size limit by default ArchiveUtil.MAX_ENTRY_SIZE (32 MB) was designed for the handful of known-small metadata files (README, package.json, icon, ...) extracted at publish time. /vscode/unpkg/{namespace}/{extension}/{version}/{path} serves arbitrary files bundled in the VSIX -- e.g. a WASM binary -- which can legitimately be much larger and were only ever bounded by the whole-package upload limit. Reusing that constant here risked rejecting a file that published successfully. ovsx.caching.files-webresource.max-file-size now defaults to -1 (unbounded), matching pre-#2060 behavior exactly: no size check, no SizeLimitInputStream wrapping. Operators who want the protection this PR introduced (bounding decompressed size against a lying or oversized zip header) can opt in with a positive byte value, same as before. --- .../openvsx/adapter/WebResourceService.java | 47 ++++++++++++------- .../adapter/WebResourceServiceTest.java | 21 +++++++++ 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java b/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java index 29a88f402..168845825 100644 --- a/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java +++ b/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java @@ -51,9 +51,14 @@ public class WebResourceService { private final FilesCacheKeyGenerator filesCacheKeyGenerator; private final JsonMapper jsonMapper; - // Limit the decompressed size of a single served web resource, default 32 MB matching - // ArchiveUtil.MAX_ENTRY_SIZE. Without this, a small VSIX containing a highly compressed entry - // could exhaust the java.io.tmpdir filesystem when that entry is extracted on request. + // Limit the decompressed size of a single served web resource. Disabled (unbounded, matching + // pre-existing behavior) by default: unlike the handful of known-small metadata files bounded by + // ArchiveUtil.MAX_ENTRY_SIZE at publish time, an arbitrary file requested through /vscode/unpkg + // can legitimately be large (e.g. a bundled WASM binary), so there's no one-size-fits-all default + // that doesn't risk rejecting something that published successfully. A negative value disables the + // limit; set a positive byte value to opt into bounding it, e.g. to guard against a small VSIX + // containing a highly compressed entry that could exhaust the java.io.tmpdir filesystem when + // extracted on request. private final long maxFileSize; public WebResourceService( @@ -61,7 +66,7 @@ public WebResourceService( RepositoryService repositories, CacheService cache, FilesCacheKeyGenerator filesCacheKeyGenerator, - @Value("${ovsx.caching.files-webresource.max-file-size:33554432}") long maxFileSize + @Value("${ovsx.caching.files-webresource.max-file-size:-1}") long maxFileSize ) { this.storageUtil = storageUtil; this.repositories = repositories; @@ -165,22 +170,30 @@ private String getFileExtension(ZipEntry fileEntry) { private void writeBinaryFile(Path file, ZipFile zip, ZipEntry fileEntry) { var declaredSize = fileEntry.getSize(); - if (declaredSize < 0) { - throw new ErrorResultException("The file " + fileEntry.getName() + " has an unknown size."); - } - if (declaredSize > maxFileSize) { - var maxSize = FileUtils.byteCountToDisplaySize(maxFileSize); - throw new ErrorResultException( - "The file " + fileEntry.getName() + " exceeds the size limit of " + maxSize + ".", - HttpStatus.CONTENT_TOO_LARGE); + if (maxFileSize >= 0) { + if (declaredSize < 0) { + throw new ErrorResultException("The file " + fileEntry.getName() + " has an unknown size."); + } + if (declaredSize > maxFileSize) { + var maxSize = FileUtils.byteCountToDisplaySize(maxFileSize); + throw new ErrorResultException( + "The file " + fileEntry.getName() + " exceeds the size limit of " + maxSize + ".", + HttpStatus.CONTENT_TOO_LARGE); + } } FileUtil.writeSync(file, p -> { - // Wrap in SizeLimitInputStream bounded to the declared size: a zip entry can lie about - // its uncompressed size, so this stops the copy the moment more bytes than declared - // have been read instead of trusting the header. - try (var in = zip.getInputStream(fileEntry); var limited = new SizeLimitInputStream(in, declaredSize)) { - Files.copy(limited, p); + try (var in = zip.getInputStream(fileEntry)) { + if (maxFileSize >= 0) { + // Wrap in SizeLimitInputStream bounded to the declared size: a zip entry can lie + // about its uncompressed size, so this stops the copy the moment more bytes than + // declared have been read instead of trusting the header. + try (var limited = new SizeLimitInputStream(in, declaredSize)) { + Files.copy(limited, p); + } + } else { + Files.copy(in, p); + } } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/server/src/test/java/org/eclipse/openvsx/adapter/WebResourceServiceTest.java b/server/src/test/java/org/eclipse/openvsx/adapter/WebResourceServiceTest.java index 17f9eba2c..8f4b729fb 100644 --- a/server/src/test/java/org/eclipse/openvsx/adapter/WebResourceServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/adapter/WebResourceServiceTest.java @@ -82,6 +82,27 @@ void testStopsCopyWhenActualBytesExceedDeclaredSize() throws Exception { assertThat(Files.exists(cachedPath)).isFalse(); } + @Test + void testAllowsLargeFileWhenLimitIsDisabled() throws Exception { + // extension/README.md declares 31781 bytes -- larger than the 8 KB limit rejected in + // testRejectsEntryDeclaredLargerThanLimit -- but a negative maxFileSize (the default) means + // no file is too large, matching the pre-existing behavior of this endpoint. + var extensionDownloadPath = resourcePath("wrong-size.zip"); + var service = newService(-1L); + + var cachedPath = service.getWebResource( + "ns", + "ext", + null, + "1.0.0", + "extension/README.md", + extensionDownloadPath); + + assertThat(cachedPath).isNotNull(); + assertThat(Files.size(cachedPath)).isEqualTo(31781); + Files.deleteIfExists(cachedPath); + } + @Test void testServesFileWithinLimit() throws Exception { var extensionDownloadPath = resourcePath("todo-tree.zip"); From 48c4ed1cdcafe5426b9d95a45a891f97e703d462 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Mon, 24 Aug 2026 14:36:40 +0200 Subject: [PATCH 4/4] ci: raise test task heap to 6144m to fix CI OutOfMemoryError CI intermittently failed with java.lang.OutOfMemoryError: Java heap space during unrelated Spring test context bootstrapping (e.g. SitemapControllerTest, JacksonConfigDtoTest) -- not caused by this PR's changes, but the same run passed twice earlier with identical content, so it's resource pressure on the existing 4096m ceiling rather than a deterministic bug. The suite caches many distinct @SpringBootTest/@WebMvcTest contexts, which adds up across the whole test class list; 4 GB was occasionally too tight for that. --- server/build.gradle | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/build.gradle b/server/build.gradle index 06f71a5a7..bee01674c 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -276,7 +276,10 @@ testlogger { } test { - jvmArgs = ['--enable-native-access=ALL-UNNAMED', '-Xmx4096m', '-Xshare:off'] // due to https://github.com/netty/netty/issues/15161 + // -Xmx raised from 4096m: the full suite caches many distinct Spring test contexts + // (@SpringBootTest/@WebMvcTest each get their own), and 4 GB was occasionally too tight, + // observed as an OutOfMemoryError during unrelated context bootstrapping on CI. + jvmArgs = ['--enable-native-access=ALL-UNNAMED', '-Xmx6144m', '-Xshare:off'] // due to https://github.com/netty/netty/issues/15161 useJUnitPlatform() }