From 60135ec04dbfa18262ede8643c4c772a451a67a2 Mon Sep 17 00:00:00 2001
From: Adnan Al <98621989+achdmbp@users.noreply.github.com>
Date: Wed, 20 May 2026 13:01:37 -0500
Subject: [PATCH] fix: cache S3Presigner as singleton to prevent credential
 refresh failure

The S3Presigner was created fresh on every getLocation() call and
immediately closed via try-with-resources. Closing the presigner shuts
down the underlying HTTP connection pool used by DefaultCredentialsProvider
for STS token refresh. After ~63 minutes when IRSA credentials expire,
the next presign attempt fails because the connection pool is dead.

Fix: Cache the S3Presigner as a singleton field (same pattern as S3Client).
The presigner stays open for the lifetime of the application, allowing
DefaultCredentialsProvider to refresh IRSA tokens via its internal HTTP
client without interruption.

Fixes: https://github.com/eclipse-openvsx/openvsx/issues/1855
Signed-off-by: Adnan Al <98621989+achdmbp@users.noreply.github.com>
---
 .../openvsx/storage/AwsStorageService.java    | 38 ++++++++++---------
 .../storage/AwsStorageServiceTest.java        | 21 ++++++++++
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/AwsStorageService.java b/server/src/main/java/org/eclipse/openvsx/storage/AwsStorageService.java
index 29bec249b..3cdeb3cfd 100644
--- a/server/src/main/java/org/eclipse/openvsx/storage/AwsStorageService.java
+++ b/server/src/main/java/org/eclipse/openvsx/storage/AwsStorageService.java
@@ -77,6 +77,7 @@ public class AwsStorageService implements IStorageService {
     boolean pathStyleAccess;
 
     private S3Client s3Client;
+    private S3Presigner s3Presigner;
 
     public AwsStorageService(FileCacheDurationConfig fileCacheDurationConfig, FilesCacheKeyGenerator filesCacheKeyGenerator) {
         this.fileCacheDurationConfig = fileCacheDurationConfig;
@@ -110,24 +111,27 @@ public S3Client getS3Client() {
     }
 
     private S3Presigner getS3Presigner() {
-        var builder = S3Presigner.builder()
-                .region(Region.of(region))
-                .credentialsProvider(getCredentialsProvider());
-
-        if(StringUtils.isNotEmpty(serviceEndpoint)) {
-            var endpointParams = S3EndpointParams.builder()
-                    .endpoint(serviceEndpoint)
+        if (s3Presigner == null) {
+            var builder = S3Presigner.builder()
                     .region(Region.of(region))
-                    .build();
+                    .credentialsProvider(getCredentialsProvider());
 
-            var endpoint = S3EndpointProvider
-                    .defaultProvider()
-                    .resolveEndpoint(endpointParams).join();
+            if(StringUtils.isNotEmpty(serviceEndpoint)) {
+                var endpointParams = S3EndpointParams.builder()
+                        .endpoint(serviceEndpoint)
+                        .region(Region.of(region))
+                        .build();
 
-            builder = builder.endpointOverride(endpoint.url());
-        }
+                var endpoint = S3EndpointProvider
+                        .defaultProvider()
+                        .resolveEndpoint(endpointParams).join();
 
-        return builder.build();
+                builder = builder.endpointOverride(endpoint.url());
+            }
+
+            s3Presigner = builder.build();
+        }
+        return s3Presigner;
     }
 
     private AwsCredentialsProvider getCredentialsProvider() {
@@ -240,10 +244,8 @@ private URI getLocation(String objectKey) {
                 .getObjectRequest(objectRequest)
                 .build();
 
-        try (var presigner = getS3Presigner()) {
-            var presignedRequest = presigner.presignGetObject(presignRequest);
-            return presignedRequest.httpRequest().getUri();
-        }
+        var presignedRequest = getS3Presigner().presignGetObject(presignRequest);
+        return presignedRequest.httpRequest().getUri();
     }
 
     @Override
diff --git a/server/src/test/java/org/eclipse/openvsx/storage/AwsStorageServiceTest.java b/server/src/test/java/org/eclipse/openvsx/storage/AwsStorageServiceTest.java
index 966663550..d79e61916 100644
--- a/server/src/test/java/org/eclipse/openvsx/storage/AwsStorageServiceTest.java
+++ b/server/src/test/java/org/eclipse/openvsx/storage/AwsStorageServiceTest.java
@@ -253,4 +253,25 @@ void testAuthenticationMethodDetection() {
         assertFalse((Boolean) ReflectionTestUtils.invokeMethod(storageService, "hasStaticCredentials"));
         assertFalse((Boolean) ReflectionTestUtils.invokeMethod(storageService, "hasSessionToken"));
     }
+
+    @Test
+    void testPresignerIsCachedAsSingleton() {
+        // Only region is needed to construct the presigner builder
+        ReflectionTestUtils.setField(storageService, "region", "us-east-1");
+        ReflectionTestUtils.setField(storageService, "serviceEndpoint", "");
+
+        var presigner1 = ReflectionTestUtils.invokeMethod(storageService, "getS3Presigner");
+        var presigner2 = ReflectionTestUtils.invokeMethod(storageService, "getS3Presigner");
+
+        assertNotNull(presigner1);
+        assertSame(presigner1, presigner2,
+                "S3Presigner must be cached as a singleton; recreating it on each call " +
+                "destroys the HTTP connection pool needed for IRSA credential refresh");
+    }
+
+    @Test
+    void testPresignerFieldInitiallyNull() {
+        assertNull(ReflectionTestUtils.getField(storageService, "s3Presigner"),
+                "S3Presigner should be lazily initialized");
+    }
 }