Fixes #40: Clear Content-Length header on error responses (#120)

Author: tomdesair

CHANGELOG.md

@@ -21,6 +21,9 @@ All notable changes to this project will be documented in this file.
 - **Default Disk-Based Locking**: `TusFileUploadService.withStoragePath(String)` now defaults to `LeaseFileLockingService` instead of `DiskLockingService` for out-of-the-box Kubernetes, container, and shared network storage compatibility. See `docs/DISK_BASED_LOCKING.md` for legacy opt-out instructions.
 - **Calibrated Retry Budget**: Extended `TusFileUploadService` lock acquisition retry budget to 8.0 seconds (40 retries x 200ms) to ensure reliable contention resolution over network storage.
 
+### Fixed
+- **Clear Content-Length on Error Responses**: Cleared `Content-Length` response header prior to invoking `HttpServletResponse.sendError(...)` during exception handling, resolving buffer conflicts and exceptions in Undertow and other servlet containers ([#40](https://github.com/tomdesair/tus-java-server/issues/40)).
+
 ### Breaking
 - **Downloads**: In order to support both the Tus protocol and RUFH protocol, the unofficial download extension will not return a HTTP status code `204` for uploads that are still in progress and will not contain the response header `Tus-Resumable`. Removed the `UploadInProgressException` class.
 

src/main/java/me/desair/tus/server/TusFileUploadService.java

@@ -496,6 +496,7 @@ public void process(
 
     } catch (TusException e) {
       log.error("Unable to lock upload for request URI " + request.getRequestURI(), e);
+      response.setHeader(HttpHeader.CONTENT_LENGTH, null);
       response.sendError(e.getStatus(), e.getMessage());
     }
   }
@@ -744,6 +745,7 @@ protected void processTusException(
       if (problemDetails != null) {
         problemDetails.writeTo(response);
       } else {
+        response.setHeader(HttpHeader.CONTENT_LENGTH, null);
         response.sendError(status, message);
       }
     }

src/main/java/me/desair/tus/server/util/TusServletResponse.java

@@ -82,9 +82,13 @@ private void recordHeader(String name, String value) {
   }
 
   private void overwriteHeader(String name, String value) {
-    List<String> values = new LinkedList<>();
-    values.add(value);
-    headers.put(name, values);
+    if (value == null) {
+      headers.remove(name);
+    } else {
+      List<String> values = new LinkedList<>();
+      values.add(value);
+      headers.put(name, values);
+    }
   }
 
   private String sanitizeHeaderValue(String value) {

src/test/java/me/desair/tus/server/AbstractITTusFileUploadService.java

@@ -121,7 +121,7 @@ public void testDisableFeature() throws Exception {
     servletRequest.addHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
 
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
     assertResponseStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
 
     reset();
@@ -131,7 +131,7 @@ public void testDisableFeature() throws Exception {
 
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
     assertResponseStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
   }
 
@@ -258,7 +258,7 @@ public void testProcessCompleteUpload() throws Exception {
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
     assertResponseStatus(HttpServletResponse.SC_NOT_FOUND);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
   }
 
   @Test
@@ -383,7 +383,7 @@ public void testTerminateViaHttpRequest() throws Exception {
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
     assertResponseStatus(HttpServletResponse.SC_NOT_FOUND);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
   }
 
   @Test
@@ -766,7 +766,7 @@ public void testProcessUploadInvalidChecksumSecondPart() throws Exception {
     // We expect the server to return a checksum mismatch error
     assertResponseStatus(460);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
 
     // Check that upload info is still from the first patch
     UploadInfo info = tusFileUploadService.getUploadInfo(location, OWNER_KEY);
@@ -814,7 +814,7 @@ public void testProcessUploadInvalidChecksumSecondPart() throws Exception {
     tusFileUploadService.process(servletRequest, servletResponse);
     assertResponseStatus(HttpServletResponse.SC_NOT_FOUND);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
   }
 
   @Test
@@ -879,7 +879,7 @@ public void testCleanupExpiredUpload() throws Exception {
 
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
     assertResponseStatus(HttpServletResponse.SC_NOT_FOUND);
   }
 
@@ -1490,7 +1490,7 @@ public void testHeadOnNonExistingUpload() throws Exception {
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
     assertResponseStatus(HttpServletResponse.SC_NOT_FOUND);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
   }
 
   @Test
@@ -1504,7 +1504,7 @@ public void testInvalidTusResumable() throws Exception {
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
     assertResponseStatus(HttpServletResponse.SC_PRECONDITION_FAILED);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
   }
 
   @Test
@@ -1523,7 +1523,7 @@ public void testMaxUploadLengthExceeded() throws Exception {
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
     assertResponseStatus(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
   }
 
   @Test
@@ -1543,7 +1543,7 @@ public void testInvalidMethods() throws Exception {
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
     assertResponseStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
 
     reset();
     servletRequest.setMethod("TRACE");
@@ -1553,7 +1553,7 @@ public void testInvalidMethods() throws Exception {
     tusFileUploadService.process(servletRequest, servletResponse, OWNER_KEY);
     assertResponseStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
     assertResponseHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
-    assertResponseHeader(HttpHeader.CONTENT_LENGTH, "0");
+    assertResponseHeaderNull(HttpHeader.CONTENT_LENGTH);
   }
 
   @Test

src/test/java/me/desair/tus/server/TusFileUploadServiceTest.java

@@ -1,6 +1,7 @@
 package me.desair.tus.server;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -168,9 +169,44 @@ public void testProcessLockFailure() throws Exception {
 
     service.process(mockReq, mockResp, "owner");
 
+    verify(mockResp, times(1)).setHeader(HttpHeader.CONTENT_LENGTH, null);
     verify(mockResp, times(1)).sendError(423, "Locked");
   }
 
+  @Test
+  public void testProcessTusExceptionClearsContentLengthHeader() throws Exception {
+    UploadLockingService mockLockingService = mock(UploadLockingService.class);
+    UploadLock mockLock = mock(UploadLock.class);
+    when(mockLockingService.lockUploadByUri(anyString())).thenReturn(mockLock);
+
+    UploadStorageService mockStorage = mock(UploadStorageService.class);
+    when(mockStorage.getUploadInfo(anyString(), anyString())).thenReturn(null);
+
+    org.springframework.mock.web.MockHttpServletRequest mockReq =
+        new org.springframework.mock.web.MockHttpServletRequest();
+    org.springframework.mock.web.MockHttpServletResponse mockResp =
+        new org.springframework.mock.web.MockHttpServletResponse();
+
+    mockReq.setMethod("PATCH");
+    mockReq.setRequestURI("/files/test");
+    mockReq.addHeader(HttpHeader.TUS_RESUMABLE, "1.0.0");
+    mockReq.addHeader(HttpHeader.CONTENT_TYPE, "application/offset+octet-stream");
+    mockReq.addHeader(HttpHeader.UPLOAD_OFFSET, "0");
+
+    TusFileUploadService service =
+        new TusFileUploadService()
+            .withUploadLockingService(mockLockingService)
+            .withUploadStorageService(mockStorage);
+
+    service.process(mockReq, mockResp, "owner");
+
+    // Check that Tus-Resumable is preserved while Content-Length is cleared for sendError
+    assertThat(mockResp.getHeader(HttpHeader.TUS_RESUMABLE), is("1.0.0"));
+    assertThat(mockResp.getHeader(HttpHeader.CONTENT_LENGTH), is(nullValue()));
+    assertThat(mockResp.getStatus(), is(404));
+    verify(mockLock, times(1)).close();
+  }
+
   @Test
   public void testProtocolVersionConfiguration() {
     TusFileUploadService service = new TusFileUploadService();

src/test/java/me/desair/tus/server/util/TusServletResponseTest.java

@@ -127,4 +127,14 @@ public void addIntHeader() throws Exception {
   public void getHeaderNull() throws Exception {
     assertThat(tusServletResponse.getHeader("TEST"), is(nullValue()));
   }
+
+  @Test
+  public void setHeaderNullClearsHeader() throws Exception {
+    tusServletResponse.setHeader("TEST", "foo");
+    assertThat(tusServletResponse.getHeader("TEST"), is("foo"));
+
+    tusServletResponse.setHeader("TEST", null);
+    assertThat(tusServletResponse.getHeader("TEST"), is(nullValue()));
+    assertThat(servletResponse.getHeader("TEST"), is(nullValue()));
+  }
 }