diff --git a/src/main/java/me/desair/tus/server/upload/UploadIdFactory.java b/src/main/java/me/desair/tus/server/upload/UploadIdFactory.java index 4a5f21c..274dd73 100644 --- a/src/main/java/me/desair/tus/server/upload/UploadIdFactory.java +++ b/src/main/java/me/desair/tus/server/upload/UploadIdFactory.java @@ -1,6 +1,8 @@ package me.desair.tus.server.upload; import java.io.Serializable; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.regex.Matcher; import java.util.regex.Pattern; import me.desair.tus.server.util.Utils; @@ -14,10 +16,13 @@ */ public abstract class UploadIdFactory { + private final ReadWriteLock lock = new ReentrantReadWriteLock(); private String uploadUri = "/"; - // volatile ensures changes made via setUploadUri(..) are immediately visible across - // multiple concurrent request threads without stale caching. - private volatile Pattern uploadUriPattern = null; + // Read and write operations on uploadUri and uploadUriPattern are guarded by a + // ReentrantReadWriteLock. + // This enables concurrent lock-free reads for high performance while ensuring thread-safe lazy + // pattern compilation and state mutation without triggering Sonar S3077 warnings. + private Pattern uploadUriPattern = null; /** * Set the URI or absolute URL under which the main tus upload endpoint is hosted. Optionally, @@ -35,8 +40,14 @@ public void setUploadUri(String uploadUri) { || Strings.CS.startsWith(uploadUri, "https://"), "The upload URI should start with /, http://, or https://"); Validate.isTrue(!Strings.CS.endsWith(uploadUri, "$"), "The upload URI should not end with $"); - this.uploadUri = uploadUri; - this.uploadUriPattern = null; + + lock.writeLock().lock(); + try { + this.uploadUri = uploadUri; + this.uploadUriPattern = null; + } finally { + lock.writeLock().unlock(); + } } /** @@ -46,7 +57,12 @@ public void setUploadUri(String uploadUri) { * @return The URI of the main tus upload endpoint. */ public String getUploadUri() { - return uploadUri; + lock.readLock().lock(); + try { + return uploadUri; + } finally { + lock.readLock().unlock(); + } } /** @@ -91,13 +107,27 @@ public UploadId readUploadId(String url) { * @return A (cached) Pattern to match upload URI's */ protected Pattern getUploadUriPattern() { - if (uploadUriPattern == null) { - // We will extract the upload ID's by removing the upload URI from the start of the - // request URI - String path = Utils.extractUriPath(uploadUri); - uploadUriPattern = - Pattern.compile("^.*" + path + (Strings.CS.endsWith(path, "/") ? "" : "/?")); + lock.readLock().lock(); + try { + if (uploadUriPattern != null) { + return uploadUriPattern; + } + } finally { + lock.readLock().unlock(); + } + + lock.writeLock().lock(); + try { + if (uploadUriPattern == null) { + // Extract upload IDs by removing upload URI from start of request URI. + // Write lock ensures single pattern compilation across concurrent threads. + String path = Utils.extractUriPath(uploadUri); + uploadUriPattern = + Pattern.compile("^.*" + path + (Strings.CS.endsWith(path, "/") ? "" : "/?")); + } + return uploadUriPattern; + } finally { + lock.writeLock().unlock(); } - return uploadUriPattern; } } diff --git a/src/test/java/me/desair/tus/server/upload/UuidUploadIdFactoryTest.java b/src/test/java/me/desair/tus/server/upload/UuidUploadIdFactoryTest.java index 5049eb7..8a2eb77 100644 --- a/src/test/java/me/desair/tus/server/upload/UuidUploadIdFactoryTest.java +++ b/src/test/java/me/desair/tus/server/upload/UuidUploadIdFactoryTest.java @@ -6,6 +6,11 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.Before; import org.junit.Test; @@ -159,4 +164,53 @@ public void readUploadIdRegexAbsoluteUrl() throws Exception { public void createId() throws Exception { assertThat(idFactory.createId(), not(nullValue())); } + + @Test + public void testConcurrentUploadUriAccessAndUpdates() throws Exception { + int threadCount = 12; + int iterations = 100; + ExecutorService executor = Executors.newFixedThreadPool(threadCount); + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch doneLatch = new CountDownLatch(threadCount); + AtomicBoolean errorOccurred = new AtomicBoolean(false); + + idFactory.setUploadUri("/test/upload"); + + for (int i = 0; i < threadCount; i++) { + final int threadId = i; + executor.submit( + () -> { + try { + startLatch.await(); + for (int j = 0; j < iterations; j++) { + if (threadId % 3 == 0) { + idFactory.setUploadUri("/test/upload" + (j % 5)); + } else if (threadId % 3 == 1) { + String uri = idFactory.getUploadUri(); + if (uri == null || !uri.startsWith("/test/upload")) { + errorOccurred.set(true); + } + } else { + UploadId id = + idFactory.readUploadId( + "/test/upload" + (j % 5) + "/1911e8a4-6939-490c-b58b-a5d70f8d91fb"); + if (id != null && !id.toString().equals("1911e8a4-6939-490c-b58b-a5d70f8d91fb")) { + errorOccurred.set(true); + } + } + } + } catch (Exception e) { + errorOccurred.set(true); + } finally { + doneLatch.countDown(); + } + }); + } + + startLatch.countDown(); + assertThat(doneLatch.await(5, TimeUnit.SECONDS), is(true)); + executor.shutdown(); + + assertThat(errorOccurred.get(), is(false)); + } }