From 012e70a47cc20efcf9fdfa71561199bfe1d1486e Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 7 Nov 2026 17:02:20 +0000 Subject: [PATCH 01/15] Test the client-side endpoints' caching of a failed response and their ETag A response to a request whose processing recorded errors should carry Cache-Control private, no-cache and no ETag, a matching If-None-Match should still get its 304, a missing builder result should not throw, and fixed evidence should give a fixed ETag. --- .../web/ClientsidePropertyServiceTests.java | 102 +++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index f819bd994..5a9edc372 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -27,9 +27,11 @@ import org.mockito.ArgumentMatcher; import fiftyone.pipeline.core.data.DataKey; +import fiftyone.pipeline.core.data.DataKeyBuilderDefault; import fiftyone.pipeline.core.data.EvidenceKeyFilter; import fiftyone.pipeline.core.data.EvidenceKeyFilterWhitelist; import fiftyone.pipeline.core.data.FlowData; +import fiftyone.pipeline.core.data.FlowError; import fiftyone.pipeline.core.flowelements.FlowElement; import fiftyone.pipeline.core.flowelements.Pipeline; import fiftyone.pipeline.javascriptbuilder.data.JavaScriptBuilderData; @@ -49,6 +51,7 @@ import java.io.StringWriter; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import javax.servlet.http.HttpServletRequest; @@ -186,8 +189,105 @@ public void JavaScript_VaryHeader_2Element_UA_AND_SEC() throws IOException { JavaScript_VaryHeader(elements, "User-Agent,Sec-ch-ua"); } + /** + * The ETag is worked out from the evidence values alone, so the same + * evidence gives the same ETag in every process. The expected value is + * the exclusive or of the values' String.hashCode, which the Java + * language defines, so it does not depend on the process. + */ + @Test + public void JavaScript_ETag_SameInEveryProcess() throws IOException { + DataKey key = new DataKeyBuilderDefault() + .add(0, "header.user-agent", + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " + + "(KHTML, like Gecko) Chrome/98.0.4758.102 Safari/537.36") + .add(0, "header.sec-ch-ua-mobile", "?0") + .build(); + when(flowData.generateKey(any(EvidenceKeyFilter.class))).thenReturn(key); + Configure(null); + + service.serveJavascript(request, response); + + verify(response).setHeader(eq("ETag"), eq("661586230")); + } + + /** + * A response built while processing recorded errors may lack the data of + * the element that failed, so it is not kept and carries no ETag to + * revalidate with. + */ + @Test + public void JavaScript_ProcessingErrors_NoCacheAndNoETag() + throws IOException { + recordProcessingError(); + Configure(null); + + service.serveJavascript(request, response); + + ValidateFailedResponse(JS_CONTENT, JS_CONTENT_TYPE); + } + + @Test + public void Json_ProcessingErrors_NoCacheAndNoETag() throws IOException { + recordProcessingError(); + Configure(null); + + service.serveJson(request, response); + + ValidateFailedResponse(JSON_CONTENT, JSON_CONTENT_TYPE); + } + + /** + * A browser holding a good response keeps it while processing fails. + */ + @Test + public void JavaScript_ProcessingErrors_MatchingIfNoneMatch_304() + throws IOException { + recordProcessingError(); + Configure(null); + when(request.getHeader("If-None-Match")) + .thenReturn(String.valueOf(dataKey.hashCode())); + + service.serveJavascript(request, response); + + verify(response).setStatus(eq(304)); + assertEquals("", responseString.toString()); + } + + /** + * When the builder element is the one that failed there is no content, + * and the response is still sent rather than throwing. + */ + @Test + public void JavaScript_ProcessingErrors_NoContent() throws IOException { + recordProcessingError(); + when(flowData.getFromElement(jsElement)).thenReturn(null); + Configure(null); + + service.serveJavascript(request, response); + + ValidateFailedResponse("", JS_CONTENT_TYPE); + } + + private void recordProcessingError() { + when(flowData.getErrors()).thenReturn( + Collections.singletonList(mock(FlowError.class))); + } + + private void ValidateFailedResponse( + String expectedContent, + String contentType) { + assertEquals(expectedContent, responseString.toString()); + verify(response).setStatus(eq(200)); + verify(response).setContentType(eq(contentType)); + verify(response).setContentLength(eq(expectedContent.length())); + verify(response).setHeader(eq("Cache-Control"), eq("private, no-cache")); + verify(response, times(0)).setHeader(eq("ETag"), anyString()); + verify(response).setHeader(eq("Access-Control-Allow-Origin"), eq("*")); + } + private void JavaScript_VaryHeader( - List> elements, + List> elements, String expectedVary) throws IOException { Configure(ConfigureElements(elements)); From b8b53dcd3831a58ec1bfe68e90b4d16b04cb689c Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 7 Nov 2026 17:03:09 +0000 Subject: [PATCH 02/15] Send no ETag and private, no-cache on a failed client-side response A response to a request whose processing recorded errors may lack the data of the element that failed. It carried the ETag and Cache-Control private,max-age=1800, so a browser kept it for 30 minutes and then revalidated it into a 304 for as long as the evidence stayed the same, although processing worked again. It now carries Cache-Control private, no-cache and no ETag, as the web integration specification asks. A request whose If-None-Match matches still gets its 304, so a browser holding a good response keeps it through an outage. A missing builder result no longer throws a NullPointerException when the content is written. --- .../ClientsidePropertyServiceCore.java | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java index 67439d7cc..9b19dc838 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java @@ -25,6 +25,7 @@ import fiftyone.pipeline.core.data.EvidenceKeyFilter; import fiftyone.pipeline.core.data.EvidenceKeyFilterWhitelist; import fiftyone.pipeline.core.data.FlowData; +import fiftyone.pipeline.core.data.FlowError; import fiftyone.pipeline.core.exceptions.PipelineConfigurationException; import fiftyone.pipeline.core.flowelements.FlowElement; import fiftyone.pipeline.core.flowelements.Pipeline; @@ -35,6 +36,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import static fiftyone.pipeline.core.Constants.EVIDENCE_HTTPHEADER_PREFIX; @@ -121,6 +123,13 @@ private enum ContentTypes { "private", "max-age=1800"); + /** + * The cache control value for a response built while processing + * recorded errors. A browser may keep it but must ask before using + * it again, and with no ETag the next request gets a full response. + */ + private static final String FAILED_CACHE_CONTROL = "private, no-cache"; + /** * Create a new instance. * @param flowDataProviderCore the provider to the {@link FlowData} for @@ -240,13 +249,22 @@ public void serveContent( break; } + // A response built while processing recorded errors may lack + // the data of the element that failed, so it must not be + // revalidated into a 304 once processing works again. + Collection errors = flowData.getErrors(); + boolean failed = errors != null && errors.isEmpty() == false; + setHeaders( response, hash, + failed, content == null ? 0 : content.length(), contentType == ContentTypes.JavaScript ? "x-javascript" : "json"); - response.getWriter().write(content); + if (content != null) { + response.getWriter().write(content); + } } } @@ -256,6 +274,8 @@ public void serveContent( * @param response the {@link HttpServletResponse} to add the response * headers to * @param hash the hash to use for the ETag + * @param failed true when processing recorded errors, in which case + * the response is not cached and carries no ETag * @param contentLength the length of the returned content. This is used * for the 'Content-Length' header * @param contentType the type of content @@ -263,14 +283,19 @@ public void serveContent( private void setHeaders( HttpServletResponse response, int hash, + boolean failed, int contentLength, String contentType) { response.setContentType("application/" + contentType); response.setContentLength(contentLength); response.setStatus(200); - response.setHeader("Cache-Control", stringJoin(cacheControl, ",")); response.setHeader("Vary", stringJoin(headersAffectingJavaScript, ",")); - response.setHeader("ETag", Integer.toString(hash)); + if (failed) { + response.setHeader("Cache-Control", FAILED_CACHE_CONTROL); + } else { + response.setHeader("Cache-Control", stringJoin(cacheControl, ",")); + response.setHeader("ETag", Integer.toString(hash)); + } response.setHeader("Access-Control-Allow-Origin", "*"); } } From d310fad116d8110563d1258a092083efc59b8805 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 7 Nov 2026 17:21:15 +0000 Subject: [PATCH 03/15] Say what a failed response's headers are in setHeaders' comment --- .../pipeline/web/services/ClientsidePropertyServiceCore.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java index 9b19dc838..c9a661998 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java @@ -274,8 +274,8 @@ public void serveContent( * @param response the {@link HttpServletResponse} to add the response * headers to * @param hash the hash to use for the ETag - * @param failed true when processing recorded errors, in which case - * the response is not cached and carries no ETag + * @param failed true when processing recorded errors, which sends + * no-cache in place of max-age, and no ETag * @param contentLength the length of the returned content. This is used * for the 'Content-Length' header * @param contentType the type of content From 4f62ea4dec448116c4c3fae1988edea331864b91 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 7 Nov 2026 21:33:47 +0000 Subject: [PATCH 04/15] Test that a matching If-None-Match gets 412 for POST and a 304 with the 200's headers RFC 9110 section 13.1.2 answers a matching If-None-Match with 304 for GET and HEAD and 412 for any other method, and section 15.4.5 has a 304 carry the Cache-Control, Vary and ETag headers a 200 would. --- .../web/ClientsidePropertyServiceTests.java | 79 ++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index 5a9edc372..4b49b4a46 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -81,6 +81,7 @@ public class ClientsidePropertyServiceTests { @Before public void init() throws IOException { request = mock(HttpServletRequest.class); + when(request.getMethod()).thenReturn("GET"); response = mock(HttpServletResponse.class); responseString = new StringWriter(); PrintWriter responseWriter = new PrintWriter(responseString); @@ -238,13 +239,56 @@ public void Json_ProcessingErrors_NoCacheAndNoETag() throws IOException { } /** - * A browser holding a good response keeps it while processing fails. + * A browser holding a good response keeps it while processing fails. The + * 304 carries the caching headers a 200 to the same request would, so the + * browser asks again next time rather than keeping it for 30 minutes. */ @Test public void JavaScript_ProcessingErrors_MatchingIfNoneMatch_304() throws IOException { recordProcessingError(); + Configure(ConfigureElements( + Collections.singletonList(Arrays.asList("header.User-Agent")))); + when(request.getHeader("If-None-Match")) + .thenReturn(String.valueOf(dataKey.hashCode())); + + service.serveJavascript(request, response); + + verify(response).setStatus(eq(304)); + assertEquals("", responseString.toString()); + verify(response).setHeader(eq("Cache-Control"), eq("private, no-cache")); + verify(response, times(0)).setHeader(eq("ETag"), anyString()); + verify(response).setHeader(eq("Vary"), eq("User-Agent")); + verify(response).setHeader(eq("Access-Control-Allow-Origin"), eq("*")); + } + + /** + * A 304 carries the Cache-Control, Vary and ETag headers a 200 to the same + * request would, as RFC 9110 section 15.4.5 requires, and the CORS header. + */ + @Test + public void JavaScript_MatchingIfNoneMatch_304_CarriesHeaders() + throws IOException { + Configure(ConfigureElements( + Collections.singletonList(Arrays.asList("header.User-Agent")))); + String etag = String.valueOf(dataKey.hashCode()); + when(request.getHeader("If-None-Match")).thenReturn(etag); + + service.serveJavascript(request, response); + + verify(response).setStatus(eq(304)); + assertEquals("", responseString.toString()); + verify(response).setHeader(eq("Cache-Control"), eq("private,max-age=1800")); + verify(response).setHeader(eq("ETag"), eq(etag)); + verify(response).setHeader(eq("Vary"), eq("User-Agent")); + verify(response).setHeader(eq("Access-Control-Allow-Origin"), eq("*")); + verify(response, times(0)).setContentLength(anyInt()); + } + + @Test + public void JavaScript_MatchingIfNoneMatch_Head_304() throws IOException { Configure(null); + when(request.getMethod()).thenReturn("HEAD"); when(request.getHeader("If-None-Match")) .thenReturn(String.valueOf(dataKey.hashCode())); @@ -254,6 +298,39 @@ public void JavaScript_ProcessingErrors_MatchingIfNoneMatch_304() assertEquals("", responseString.toString()); } + /** + * RFC 9110 section 13.1.2 answers a matching If-None-Match with 412 for + * any method other than GET or HEAD, such as the POST the client-side + * script sends to the JSON endpoint. + */ + @Test + public void Json_MatchingIfNoneMatch_Post_412() throws IOException { + Configure(null); + when(request.getMethod()).thenReturn("POST"); + when(request.getHeader("If-None-Match")) + .thenReturn(String.valueOf(dataKey.hashCode())); + + service.serveJson(request, response); + + verify(response).setStatus(eq(412)); + verify(response, times(0)).setStatus(eq(304)); + assertEquals("", responseString.toString()); + } + + /** + * Without a matching If-None-Match a POST gets the JSON as before. + */ + @Test + public void Json_Post_200() throws IOException { + Configure(null); + when(request.getMethod()).thenReturn("POST"); + + service.serveJson(request, response); + + verify(response).setStatus(eq(200)); + assertEquals(JSON_CONTENT, responseString.toString()); + } + /** * When the builder element is the one that failed there is no content, * and the response is still sent rather than throwing. From 6a74ed166f29fd8d1591f95de59d1c92378117a3 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 7 Nov 2026 21:35:01 +0000 Subject: [PATCH 05/15] Answer a matching If-None-Match with 412 for POST, and give the 304 the 200's headers RFC 9110 section 13.1.2 has an origin server answer a matching If-None-Match with 304 for GET and HEAD and 412 for any other method, and section 15.4.5 has the 304 carry the Cache-Control, Vary and ETag headers a 200 to the same request would. The endpoints answered every method with a bare 304. A 304 now carries Vary, Cache-Control, the ETag unless processing recorded errors, and the CORS header, so a browser that revalidates during a failure is told to ask again next time. Any method other than GET or HEAD gets 412. --- .../ClientsidePropertyServiceCore.java | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java index c9a661998..89b98691c 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java @@ -207,11 +207,25 @@ public void serveContent( FlowData flowData = flowDataProviderCore.getFlowData(request); int hash = flowData.generateKey(pipeline.getEvidenceKeyFilter()).hashCode(); + // A response built while processing recorded errors may lack the + // data of the element that failed, so it must not be revalidated + // into a 304 once processing works again. + Collection errors = flowData.getErrors(); + boolean failed = errors != null && errors.isEmpty() == false; + String ifNoneMatch = request.getHeader("If-None-Match"); if (ifNoneMatch != null && - Integer.toString(hash).equals(request.getHeader("If-None-Match"))) { - // The response hasn't changed so respond with a 304. - response.setStatus(304); + Integer.toString(hash).equals(ifNoneMatch)) { + // The content has not changed. RFC 9110 section 13.1.2 has a + // GET or HEAD answered with 304, carrying the caching headers + // of the 200 it stands for, and any other method with 412. + String method = request.getMethod(); + if ("GET".equals(method) || "HEAD".equals(method)) { + response.setStatus(304); + setCachingHeaders(response, hash, failed); + } else { + response.setStatus(412); + } } else { // Otherwise, return the requested content to the client. String content = null; @@ -249,12 +263,6 @@ public void serveContent( break; } - // A response built while processing recorded errors may lack - // the data of the element that failed, so it must not be - // revalidated into a 304 once processing works again. - Collection errors = flowData.getErrors(); - boolean failed = errors != null && errors.isEmpty() == false; - setHeaders( response, hash, @@ -289,6 +297,23 @@ private void setHeaders( response.setContentType("application/" + contentType); response.setContentLength(contentLength); response.setStatus(200); + setCachingHeaders(response, hash, failed); + } + + /** + * Set the headers a 200 and the 304 that stands for it both carry, + * which are Vary, Cache-Control, the ETag unless processing recorded + * errors, and the CORS header. + * @param response the {@link HttpServletResponse} to add the response + * headers to + * @param hash the hash to use for the ETag + * @param failed true when processing recorded errors, which sends + * no-cache in place of max-age, and no ETag + */ + private void setCachingHeaders( + HttpServletResponse response, + int hash, + boolean failed) { response.setHeader("Vary", stringJoin(headersAffectingJavaScript, ",")); if (failed) { response.setHeader("Cache-Control", FAILED_CACHE_CONTROL); From 411c8fef14f33968f0aff2146af96c7a598b25f4 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sun, 8 Nov 2026 23:49:06 +0000 Subject: [PATCH 06/15] Test that the 412 carries the CORS header --- .../fiftyone/pipeline/web/ClientsidePropertyServiceTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index 4b49b4a46..827465a6e 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -314,6 +314,7 @@ public void Json_MatchingIfNoneMatch_Post_412() throws IOException { verify(response).setStatus(eq(412)); verify(response, times(0)).setStatus(eq(304)); + verify(response).setHeader(eq("Access-Control-Allow-Origin"), eq("*")); assertEquals("", responseString.toString()); } From ade7e98155e00f893c954889fa18ee3a875af094 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sun, 8 Nov 2026 23:54:02 +0000 Subject: [PATCH 07/15] Send the CORS header with the 412 as the 304 and the 200 do --- .../pipeline/web/services/ClientsidePropertyServiceCore.java | 1 + 1 file changed, 1 insertion(+) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java index 89b98691c..2d89adff6 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java @@ -225,6 +225,7 @@ public void serveContent( setCachingHeaders(response, hash, failed); } else { response.setStatus(412); + response.setHeader("Access-Control-Allow-Origin", "*"); } } else { // Otherwise, return the requested content to the client. From 1f92896d519ec40760226b74ab294eb3802da9ce Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 19:09:40 +0000 Subject: [PATCH 08/15] Test that a revalidation gets a 304 when a Flow Element adds Evidence the Pipeline accepts The ETag is made after processing, so a session id added during processing changes it on every request. The test fails until the ETag is made from the Evidence the request supplied. --- .../web/ClientsidePropertyServiceTests.java | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index 827465a6e..6a7f3d49a 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -24,15 +24,22 @@ import org.junit.Before; import org.junit.Test; +import org.mockito.AdditionalAnswers; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; +import org.slf4j.LoggerFactory; import fiftyone.pipeline.core.data.DataKey; import fiftyone.pipeline.core.data.DataKeyBuilderDefault; import fiftyone.pipeline.core.data.EvidenceKeyFilter; +import fiftyone.pipeline.core.data.ElementData; +import fiftyone.pipeline.core.data.ElementPropertyMetaData; import fiftyone.pipeline.core.data.EvidenceKeyFilterWhitelist; import fiftyone.pipeline.core.data.FlowData; import fiftyone.pipeline.core.data.FlowError; import fiftyone.pipeline.core.flowelements.FlowElement; +import fiftyone.pipeline.core.flowelements.FlowElementBase; +import fiftyone.pipeline.core.flowelements.PipelineBuilder; import fiftyone.pipeline.core.flowelements.Pipeline; import fiftyone.pipeline.javascriptbuilder.data.JavaScriptBuilderData; import fiftyone.pipeline.javascriptbuilder.flowelements.JavaScriptBuilderElement; @@ -347,6 +354,101 @@ public void JavaScript_ProcessingErrors_NoContent() throws IOException { ValidateFailedResponse("", JS_CONTENT_TYPE); } + /** + * The ETag is made from the evidence the request supplied. A Flow Element + * that accepts the evidence it adds, as the Sequence Element does with a + * new session id, would otherwise give every request a different ETag, and + * a revalidation would never get a 304. + */ + @Test + public void JavaScript_ElementAddsEvidence_RevalidationGets304() + throws Exception { + try (Pipeline real = new PipelineBuilder() + .addFlowElement(new SessionIdElement()) + .build()) { + pipeline = mock(Pipeline.class); + when(pipeline.getFlowElements()).thenReturn(real.getFlowElements()); + when(pipeline.getElement(JavaScriptBuilderElement.class)).thenReturn(jsElement); + when(pipeline.getElement(JsonBuilderElement.class)).thenReturn(jsonElement); + when(pipeline.getEvidenceKeyFilter()).thenReturn(real.getEvidenceKeyFilter()); + service = new ClientsidePropertyServiceCore.Default( + flowDataProvider, pipeline); + + ArgumentCaptor eTag = ArgumentCaptor.forClass(String.class); + try (FlowData first = real.createFlowData()) { + first.process(); + FlowData wrapped = wrap(first); + when(flowDataProvider.getFlowData(any(HttpServletRequest.class))) + .thenReturn(wrapped); + service.serveJavascript(request, response); + verify(response).setHeader(eq("ETag"), eTag.capture()); + } + + when(request.getHeader("If-None-Match")).thenReturn(eTag.getValue()); + try (FlowData second = real.createFlowData()) { + second.process(); + FlowData wrapped = wrap(second); + when(flowDataProvider.getFlowData(any(HttpServletRequest.class))) + .thenReturn(wrapped); + service.serveJavascript(request, response); + } + + verify(response).setStatus(304); + } + } + + private FlowData wrap(FlowData data) throws Exception { + FlowData wrapped = mock( + FlowData.class, AdditionalAnswers.delegatesTo(data)); + doReturn(pipeline).when(wrapped).getPipeline(); + doReturn(jsData).when(wrapped).getFromElement(jsElement); + doReturn(jsonData).when(wrapped).getFromElement(jsonElement); + return wrapped; + } + + /** + * Accepts and adds a session id as the Sequence Element does. + */ + private static class SessionIdElement + extends FlowElementBase { + + private static final String SESSION_ID = "query.session-id"; + + SessionIdElement() { + super(LoggerFactory.getLogger(SessionIdElement.class), null); + } + + @Override + protected void processInternal(FlowData data) { + if (data.getEvidence().asKeyMap().containsKey(SESSION_ID) == false) { + data.addEvidence(SESSION_ID, java.util.UUID.randomUUID().toString()); + } + } + + @Override + public String getElementDataKey() { + return "session-id-test"; + } + + @Override + public fiftyone.pipeline.core.data.EvidenceKeyFilter getEvidenceKeyFilter() { + return new EvidenceKeyFilterWhitelist(Arrays.asList(SESSION_ID)); + } + + @Override + public List getProperties() { + return new ArrayList<>(); + } + + @Override + protected void managedResourcesCleanup() { + } + + @Override + protected void unmanagedResourcesCleanup() { + } + } + private void recordProcessingError() { when(flowData.getErrors()).thenReturn( Collections.singletonList(mock(FlowError.class))); From 36f2526e15f627242ea33503be6f8a9ea2c1599a Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 19:10:54 +0000 Subject: [PATCH 09/15] Calculate the client-side ETag from the Evidence the request supplied A Flow Element can add or replace Evidence while processing, and a session id added that way changed the ETag on every request, so a revalidation never got a 304. FlowData now keeps a copy of the Evidence taken when processing starts, generateSuppliedKey builds a key from it, and the ETag uses that key. generateSuppliedKey is a default method, so existing FlowData implementations still compile. --- .../fiftyone/pipeline/core/data/FlowData.java | 14 +++++++++++ .../core/flowelements/FlowDataDefault.java | 23 ++++++++++++++++++- .../ClientsidePropertyServiceCore.java | 9 +++++--- .../web/ClientsidePropertyServiceTests.java | 4 ++-- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/data/FlowData.java b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/data/FlowData.java index 763da2bb3..7e1d9130f 100644 --- a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/data/FlowData.java +++ b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/data/FlowData.java @@ -276,6 +276,20 @@ T getOrAdd( */ DataKey generateKey(EvidenceKeyFilter filter); + /** + * Generate a {@link DataKey} from the evidence as it was when processing + * started, without the values that Flow Elements added or replaced while + * processing. Before processing starts it is the same as + * {@link #generateKey(EvidenceKeyFilter)}. It is for work that depends on + * the request alone, such as a cache validator. + * @param filter an {@link EvidenceKeyFilter} instance that defines the + * values to include in the generated key + * @return a new {@link DataKey} instance + */ + default DataKey generateSuppliedKey(EvidenceKeyFilter filter) { + return generateKey(filter); + } + /** * Get a filter that will only include the evidence keys that can be used by * the elements within the pipeline that created this flow element. diff --git a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java index f8758e126..7a19dce0a 100644 --- a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java +++ b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java @@ -60,6 +60,13 @@ class FlowDataDefault implements FlowData { private final AtomicBoolean processed = new AtomicBoolean(false); + /** + * A copy of the evidence taken when processing started, which leaves out + * any value a Flow Element adds or replaces while processing. Null until + * then. + */ + private volatile Map suppliedEvidence; + FlowDataDefault(Logger logger, Pipeline pipeline, Evidence evidence) { this.logger = logger; this.pipeline = (PipelineInternal) pipeline; @@ -86,6 +93,7 @@ public void process() { } else if (pipeline.isClosed()) { throw new IllegalStateException("Pipeline has been closed."); } else { + suppliedEvidence = new HashMap<>(evidence.asKeyMap()); pipeline.process(this); } } @@ -364,7 +372,20 @@ public Map getWhere(PropertyMatcher matcher) { @Override public DataKey generateKey(EvidenceKeyFilter filter) { - Map evidenceMap = evidence.asKeyMap(); + return generateKey(filter, evidence.asKeyMap()); + } + + @Override + public DataKey generateSuppliedKey(EvidenceKeyFilter filter) { + Map supplied = suppliedEvidence; + return generateKey( + filter, + supplied != null ? supplied : evidence.asKeyMap()); + } + + private static DataKey generateKey( + EvidenceKeyFilter filter, + Map evidenceMap) { DataKeyBuilder builder = new DataKeyBuilderDefault(); for (String key : evidenceMap.keySet()) { if (filter.include(key)) { diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java index 2d89adff6..468783653 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java @@ -203,9 +203,12 @@ public void serveContent( HttpServletRequest request, HttpServletResponse response, ContentTypes contentType) throws IOException { - // Get the hash code. - FlowData flowData = flowDataProviderCore.getFlowData(request); - int hash = flowData.generateKey(pipeline.getEvidenceKeyFilter()).hashCode(); + // Get the hash code from the evidence the request supplied. A + // Flow Element can add evidence while processing, such as a new + // session id, which would give every request a different ETag. + FlowData flowData = flowDataProviderCore.getFlowData(request); + int hash = flowData.generateSuppliedKey( + pipeline.getEvidenceKeyFilter()).hashCode(); // A response built while processing recorded errors may lack the // data of the element that failed, so it must not be revalidated diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index 6a7f3d49a..7576b7e95 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -99,7 +99,7 @@ public void init() throws IOException { flowData = mock(FlowData.class); dataKey = mock(DataKey.class); - when(flowData.generateKey(any(EvidenceKeyFilter.class))).thenReturn(dataKey); + when(flowData.generateSuppliedKey(any(EvidenceKeyFilter.class))).thenReturn(dataKey); jsData = mock(JavaScriptBuilderData.class); when(jsData.getJavaScript()).thenReturn(JS_CONTENT); when(flowData.getFromElement(jsElement)).thenReturn(jsData); @@ -211,7 +211,7 @@ public void JavaScript_ETag_SameInEveryProcess() throws IOException { "(KHTML, like Gecko) Chrome/98.0.4758.102 Safari/537.36") .add(0, "header.sec-ch-ua-mobile", "?0") .build(); - when(flowData.generateKey(any(EvidenceKeyFilter.class))).thenReturn(key); + when(flowData.generateSuppliedKey(any(EvidenceKeyFilter.class))).thenReturn(key); Configure(null); service.serveJavascript(request, response); From f42344ab911c60b6af3532f07aad80b37a0dcb43 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 19:57:51 +0000 Subject: [PATCH 10/15] Test the weak ETag of the supplied Evidence, If-None-Match lists, Vary and Content-Length The ETag is a weak entity tag holding a quoted SHA-256 digest, the same for the same Evidence in two Pipelines and against a value worked out apart from the Java code, and unchanged when the Sequence Element adds a new session id. If-None-Match is evaluated as RFC 9110 section 13.1.2 says, with "*", lists over one or several field lines, W/ on either side, tabs and empty elements matching, and a tag then a non-tag, two tags with no comma, a space inside the quotes, "*" with a tag, a lower case w/ and an unquoted value not matching. The JSON endpoint gets 304 for GET and 412 for POST, also while processing fails. A parallel group's headers are in Vary, and Content-Length counts bytes in the response's character encoding. The usage sharing test of a revalidation now checks the 304 carries the ETag that was sent, rather than how the ETag is worked out. --- .../web/ClientsidePropertyServiceTests.java | 519 +++++++++++++++++- .../web/UsageSharingEvidenceTests.java | 10 +- 2 files changed, 494 insertions(+), 35 deletions(-) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index 827465a6e..e2e294d4b 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -24,25 +24,37 @@ import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; +import org.slf4j.LoggerFactory; import fiftyone.pipeline.core.data.DataKey; -import fiftyone.pipeline.core.data.DataKeyBuilderDefault; +import fiftyone.pipeline.core.data.ElementData; +import fiftyone.pipeline.core.data.ElementPropertyMetaData; +import fiftyone.pipeline.core.data.Evidence; import fiftyone.pipeline.core.data.EvidenceKeyFilter; +import fiftyone.pipeline.core.data.EvidenceKeyFilterAggregator; import fiftyone.pipeline.core.data.EvidenceKeyFilterWhitelist; import fiftyone.pipeline.core.data.FlowData; import fiftyone.pipeline.core.data.FlowError; import fiftyone.pipeline.core.flowelements.FlowElement; +import fiftyone.pipeline.core.flowelements.FlowElementBase; import fiftyone.pipeline.core.flowelements.Pipeline; +import fiftyone.pipeline.core.flowelements.PipelineBuilder; +import fiftyone.pipeline.engines.fiftyone.flowelements.SequenceElementBuilder; import fiftyone.pipeline.javascriptbuilder.data.JavaScriptBuilderData; import fiftyone.pipeline.javascriptbuilder.flowelements.JavaScriptBuilderElement; import fiftyone.pipeline.jsonbuilder.data.JsonBuilderData; import fiftyone.pipeline.jsonbuilder.flowelements.JsonBuilderElement; +import fiftyone.pipeline.jsonbuilder.flowelements.JsonBuilderElementBuilder; import fiftyone.pipeline.web.services.ClientsidePropertyServiceCore; import fiftyone.pipeline.web.services.FlowDataProviderCore; +import fiftyone.pipeline.web.services.WebRequestEvidenceServiceCore; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -52,7 +64,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -64,6 +78,7 @@ public class ClientsidePropertyServiceTests { private Pipeline pipeline; private FlowData flowData; private DataKey dataKey; + private Map evidence; private JavaScriptBuilderElement jsElement; private JsonBuilderElement jsonElement; private JavaScriptBuilderData jsData; @@ -78,6 +93,31 @@ public class ClientsidePropertyServiceTests { private static final String JS_CONTENT_TYPE = "application/x-javascript"; private static final String JSON_CONTENT_TYPE = "application/json"; + private static final String USER_AGENT = + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " + + "(KHTML, like Gecko) Chrome/98.0.4758.102 Safari/537.36"; + + /** + * A weak entity tag holding a quoted SHA-256 digest in lower case hex. + */ + private static final String WEAK_ETAG = "W/\"[0-9a-f]{64}\""; + + /** + * The SHA-256 digest of the keys and values below, sorted by key, each + * written as its length in four bytes, big endian, then its UTF-8 bytes, + * worked out apart from the Java code. + * header.sec-ch-ua-mobile = ?0 + * header.user-agent = USER_AGENT + */ + private static final String ETAG_UA_AND_MOBILE = + "W/\"9beee8189f1e05e5f821379597c3a690e612c465dc53d67de2e42ebffbd4dbd3\""; + + /** + * As above, for header.user-agent = USER_AGENT alone. + */ + private static final String ETAG_UA = + "W/\"4eee715bd966ea5198b7d696aea05fd627dfd2df7f8be928e95797155ec2cf00\""; + @Before public void init() throws IOException { request = mock(HttpServletRequest.class); @@ -93,6 +133,10 @@ public void init() throws IOException { flowData = mock(FlowData.class); dataKey = mock(DataKey.class); when(flowData.generateKey(any(EvidenceKeyFilter.class))).thenReturn(dataKey); + evidence = new HashMap<>(); + Evidence flowEvidence = mock(Evidence.class); + when(flowEvidence.asKeyMap()).thenReturn(evidence); + when(flowData.getEvidence()).thenReturn(flowEvidence); jsData = mock(JavaScriptBuilderData.class); when(jsData.getJavaScript()).thenReturn(JS_CONTENT); when(flowData.getFromElement(jsElement)).thenReturn(jsData); @@ -191,25 +235,127 @@ public void JavaScript_VaryHeader_2Element_UA_AND_SEC() throws IOException { } /** - * The ETag is worked out from the evidence values alone, so the same - * evidence gives the same ETag in every process. The expected value is - * the exclusive or of the values' String.hashCode, which the Java - * language defines, so it does not depend on the process. + * The headers of the elements in a parallel group are in the Pipeline's + * Evidence key filter, so they are in the Vary header too. + */ + @Test + @SuppressWarnings("rawtypes") + public void JavaScript_VaryHeader_ParallelElements() throws IOException { + EvidenceKeyFilterAggregator group = new EvidenceKeyFilterAggregator(); + group.addFilter(new EvidenceKeyFilterWhitelist( + Arrays.asList("header.User-Agent"), + String.CASE_INSENSITIVE_ORDER)); + FlowElement element = mock(FlowElement.class); + when(element.getEvidenceKeyFilter()).thenReturn(group); + List elements = new ArrayList<>(); + elements.add(element); + Configure(elements); + + service.serveJavascript(request, response); + + verify(response).setHeader(eq("Vary"), eq("User-Agent")); + } + + /** + * The ETag is a weak entity tag, because the content can change for the + * same Evidence, for example when a data file is updated, and RFC 9110 + * section 8.8.3 asks for such a validator to be marked weak. Its opaque + * part is a quoted hash. + */ + @Test + public void JavaScript_ETag_IsWeakEntityTag() throws IOException { + Configure(null); + + service.serveJavascript(request, response); + + verify(response).setHeader(eq("ETag"), matches(WEAK_ETAG)); + } + + @Test + public void Json_ETag_IsWeakEntityTag() throws IOException { + Configure(null); + + service.serveJson(request, response); + + verify(response).setHeader(eq("ETag"), matches(WEAK_ETAG)); + } + + /** + * The same Evidence gives the same ETag in every process, so the ETag is + * a digest with no per-process seed, compared here with a value worked + * out apart from the Java code. Evidence outside the Pipeline's Evidence + * key filter, and the case of a key, leave it unchanged. */ @Test public void JavaScript_ETag_SameInEveryProcess() throws IOException { - DataKey key = new DataKeyBuilderDefault() - .add(0, "header.user-agent", - "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " + - "(KHTML, like Gecko) Chrome/98.0.4758.102 Safari/537.36") - .add(0, "header.sec-ch-ua-mobile", "?0") - .build(); - when(flowData.generateKey(any(EvidenceKeyFilter.class))).thenReturn(key); + evidence.put("header.User-Agent", USER_AGENT); + evidence.put("header.sec-ch-ua-mobile", "?0"); + evidence.put("header.X-Not-Accepted", "anything"); Configure(null); + when(pipeline.getEvidenceKeyFilter()).thenReturn( + new EvidenceKeyFilterWhitelist( + Arrays.asList("header.user-agent", "header.sec-ch-ua-mobile"), + String.CASE_INSENSITIVE_ORDER)); service.serveJavascript(request, response); - verify(response).setHeader(eq("ETag"), eq("661586230")); + verify(response).setHeader(eq("ETag"), eq(ETAG_UA_AND_MOBILE)); + } + + /** + * Two Pipelines built the same way, as in two processes, give the same + * ETag for the same request. + */ + @Test + public void Json_ETag_SameAcrossPipelines() throws Exception { + try (Pipeline first = webPipeline(); + Pipeline second = webPipeline(); + FlowData firstData = first.createFlowData(); + FlowData secondData = second.createFlowData()) { + String firstETag = + servedETag(first, firstData, webRequest(USER_AGENT)); + String secondETag = + servedETag(second, secondData, webRequest(USER_AGENT)); + + assertEquals("Two Pipelines gave different ETags.", + firstETag, secondETag); + assertEquals(ETAG_UA, firstETag); + } + } + + /** + * The Sequence Element adds a new session id to every request that has + * none, so the ETag is made from the Evidence as the request supplied it, + * and two requests with the same Evidence get the same ETag. A different + * User-Agent gives a different one. + */ + @Test + public void Json_ETag_UnchangedWhenSequenceElementAddsSessionId() + throws Exception { + try (Pipeline webPipeline = webPipeline(); + FlowData firstData = webPipeline.createFlowData(); + FlowData secondData = webPipeline.createFlowData(); + FlowData otherData = webPipeline.createFlowData()) { + String firstETag = + servedETag(webPipeline, firstData, webRequest(USER_AGENT)); + String secondETag = + servedETag(webPipeline, secondData, webRequest(USER_AGENT)); + String otherETag = + servedETag(webPipeline, otherData, webRequest("Other")); + + Object firstSession = + firstData.getEvidence().asKeyMap().get("query.session-id"); + Object secondSession = + secondData.getEvidence().asKeyMap().get("query.session-id"); + assertNotNull("The Sequence Element added no session id.", + firstSession); + assertNotEquals("The Sequence Element added the same session id.", + firstSession, secondSession); + assertEquals("The ETag changed with the session id.", + firstETag, secondETag); + assertNotEquals("The ETag did not change with the User-Agent.", + firstETag, otherETag); + } } /** @@ -246,11 +392,11 @@ public void Json_ProcessingErrors_NoCacheAndNoETag() throws IOException { @Test public void JavaScript_ProcessingErrors_MatchingIfNoneMatch_304() throws IOException { - recordProcessingError(); Configure(ConfigureElements( Collections.singletonList(Arrays.asList("header.User-Agent")))); - when(request.getHeader("If-None-Match")) - .thenReturn(String.valueOf(dataKey.hashCode())); + String etag = etagSent(); + recordProcessingError(); + ifNoneMatch(etag); service.serveJavascript(request, response); @@ -262,6 +408,26 @@ public void JavaScript_ProcessingErrors_MatchingIfNoneMatch_304() verify(response).setHeader(eq("Access-Control-Allow-Origin"), eq("*")); } + /** + * A matching If-None-Match gets the same status while processing fails as + * at any other time, so a POST gets 412. + */ + @Test + public void Json_ProcessingErrors_MatchingIfNoneMatch_Post_412() + throws IOException { + Configure(null); + String etag = etagSent(); + recordProcessingError(); + when(request.getMethod()).thenReturn("POST"); + ifNoneMatch(etag); + + service.serveJson(request, response); + + verify(response).setStatus(eq(412)); + verify(response, times(0)).setHeader(eq("ETag"), anyString()); + assertEquals("", responseString.toString()); + } + /** * A 304 carries the Cache-Control, Vary and ETag headers a 200 to the same * request would, as RFC 9110 section 15.4.5 requires, and the CORS header. @@ -271,26 +437,33 @@ public void JavaScript_MatchingIfNoneMatch_304_CarriesHeaders() throws IOException { Configure(ConfigureElements( Collections.singletonList(Arrays.asList("header.User-Agent")))); - String etag = String.valueOf(dataKey.hashCode()); - when(request.getHeader("If-None-Match")).thenReturn(etag); + String etag = etagSent(); + ifNoneMatch(etag); service.serveJavascript(request, response); - verify(response).setStatus(eq(304)); - assertEquals("", responseString.toString()); - verify(response).setHeader(eq("Cache-Control"), eq("private,max-age=1800")); - verify(response).setHeader(eq("ETag"), eq(etag)); - verify(response).setHeader(eq("Vary"), eq("User-Agent")); - verify(response).setHeader(eq("Access-Control-Allow-Origin"), eq("*")); - verify(response, times(0)).setContentLength(anyInt()); + Validate304(etag); + } + + @Test + public void Json_MatchingIfNoneMatch_Get_304_CarriesHeaders() + throws IOException { + Configure(ConfigureElements( + Collections.singletonList(Arrays.asList("header.User-Agent")))); + String etag = etagSent(); + ifNoneMatch(etag); + + service.serveJson(request, response); + + Validate304(etag); } @Test public void JavaScript_MatchingIfNoneMatch_Head_304() throws IOException { Configure(null); + String etag = etagSent(); when(request.getMethod()).thenReturn("HEAD"); - when(request.getHeader("If-None-Match")) - .thenReturn(String.valueOf(dataKey.hashCode())); + ifNoneMatch(etag); service.serveJavascript(request, response); @@ -306,9 +479,9 @@ public void JavaScript_MatchingIfNoneMatch_Head_304() throws IOException { @Test public void Json_MatchingIfNoneMatch_Post_412() throws IOException { Configure(null); + String etag = etagSent(); when(request.getMethod()).thenReturn("POST"); - when(request.getHeader("If-None-Match")) - .thenReturn(String.valueOf(dataKey.hashCode())); + ifNoneMatch(etag); service.serveJson(request, response); @@ -318,6 +491,19 @@ public void Json_MatchingIfNoneMatch_Post_412() throws IOException { assertEquals("", responseString.toString()); } + @Test + public void JavaScript_MatchingIfNoneMatch_Post_412() throws IOException { + Configure(null); + String etag = etagSent(); + when(request.getMethod()).thenReturn("POST"); + ifNoneMatch(etag); + + service.serveJavascript(request, response); + + verify(response).setStatus(eq(412)); + assertEquals("", responseString.toString()); + } + /** * Without a matching If-None-Match a POST gets the JSON as before. */ @@ -332,6 +518,131 @@ public void Json_Post_200() throws IOException { assertEquals(JSON_CONTENT, responseString.toString()); } + /** + * "*" matches, as the endpoint always has a representation. RFC 9110 + * section 13.1.2 step 1. + */ + @Test + public void IfNoneMatch_Star_304() throws IOException { + Configure(null); + validateIfNoneMatch(304, " * "); + } + + /** + * A list matches when one of its tags matches. Step 2. + */ + @Test + public void IfNoneMatch_ListHoldingTheTag_304() throws IOException { + Configure(null); + validateIfNoneMatch(304, "W/\"0\", " + etagSent() + ", \"1\""); + } + + /** + * The comparison is weak, so the tag matches without W/ as well. + * RFC 9110 section 8.8.3.2. + */ + @Test + public void IfNoneMatch_TagWithoutWeakPrefix_304() throws IOException { + Configure(null); + validateIfNoneMatch(304, etagSent().substring(2)); + } + + /** + * List elements may be separated by tabs as well as spaces, and empty + * elements are ignored. RFC 9110 sections 5.6.1.2 and 5.6.3. + */ + @Test + public void IfNoneMatch_TabBetweenElements_304() throws IOException { + Configure(null); + validateIfNoneMatch(304, ",W/\"0\",\t" + etagSent() + "\t, ,"); + } + + /** + * Several field lines are one list. RFC 9110 section 5.2. + */ + @Test + public void IfNoneMatch_SeveralFieldLines_304() throws IOException { + Configure(null); + validateIfNoneMatch(304, "W/\"0\"", etagSent()); + } + + @Test + public void IfNoneMatch_OtherTag_200() throws IOException { + Configure(null); + validateIfNoneMatch(200, "W/\"0\""); + } + + /** + * A field that is neither "*" nor a list of entity tags does not match, + * even when it holds the tag. Step 3. + */ + @Test + public void IfNoneMatch_TagThenNonTag_200() throws IOException { + Configure(null); + validateIfNoneMatch(200, etagSent() + ", other"); + } + + @Test + public void IfNoneMatch_TwoTagsWithNoComma_200() throws IOException { + Configure(null); + validateIfNoneMatch(200, "W/\"0\" " + etagSent()); + } + + /** + * A space is not allowed inside an entity tag's quotes. RFC 9110 section + * 8.8.3. + */ + @Test + public void IfNoneMatch_SpaceInsideQuotes_200() throws IOException { + Configure(null); + validateIfNoneMatch(200, "W/\"0 1\", " + etagSent()); + } + + /** + * "*" in a list with other values is not allowed. RFC 9110 section + * 13.1.2. + */ + @Test + public void IfNoneMatch_StarAndTag_200() throws IOException { + Configure(null); + validateIfNoneMatch(200, "*", etagSent()); + } + + /** + * The weak prefix is case sensitive. RFC 9110 section 8.8.3. + */ + @Test + public void IfNoneMatch_LowerCaseWeakPrefix_200() throws IOException { + Configure(null); + validateIfNoneMatch(200, "w" + etagSent().substring(1)); + } + + /** + * An opaque tag is quoted. RFC 9110 section 8.8.3. + */ + @Test + public void IfNoneMatch_Unquoted_200() throws IOException { + Configure(null); + String etag = etagSent(); + validateIfNoneMatch(200, etag.substring(3, etag.length() - 1)); + } + + /** + * The Content-Length header is the length of the content in bytes, in + * the character encoding the response is written with. + */ + @Test + public void Json_ContentLength_InBytes() throws IOException { + String json = "{\"city\":\"Zürich\"}"; + when(jsonData.getJson()).thenReturn(json); + when(response.getCharacterEncoding()).thenReturn("UTF-8"); + Configure(null); + + service.serveJson(request, response); + + verify(response).setContentLength(eq(json.length() + 1)); + } + /** * When the builder element is the one that failed there is no content, * and the response is still sent rather than throwing. @@ -347,6 +658,154 @@ public void JavaScript_ProcessingErrors_NoContent() throws IOException { ValidateFailedResponse("", JS_CONTENT_TYPE); } + /** + * The ETag the JavaScript endpoint sends for the request as it stands, + * from a response of its own. + */ + private String etagSent() throws IOException { + HttpServletResponse sent = mock(HttpServletResponse.class); + when(sent.getWriter()).thenReturn(new PrintWriter(new StringWriter())); + service.serveJavascript(request, sent); + ArgumentCaptor etag = ArgumentCaptor.forClass(String.class); + verify(sent).setHeader(eq("ETag"), etag.capture()); + return etag.getValue(); + } + + /** + * Give the request one If-None-Match field line for each value. getHeader + * gives the first line, as a servlet container does. + */ + private void ifNoneMatch(String... lines) { + when(request.getHeaders("If-None-Match")).thenAnswer( + invocation -> Collections.enumeration(Arrays.asList(lines))); + when(request.getHeader("If-None-Match")).thenReturn(lines[0]); + } + + private void validateIfNoneMatch(int expectedStatus, String... lines) + throws IOException { + ifNoneMatch(lines); + + service.serveJavascript(request, response); + + verify(response).setStatus(eq(expectedStatus)); + assertEquals( + expectedStatus == 304 ? "" : JS_CONTENT, + responseString.toString()); + } + + private void Validate304(String etag) { + verify(response).setStatus(eq(304)); + verify(response, times(0)).setStatus(eq(200)); + assertEquals("", responseString.toString()); + verify(response).setHeader( + eq("Cache-Control"), eq("private,max-age=1800")); + verify(response).setHeader(eq("ETag"), eq(etag)); + verify(response).setHeader(eq("Vary"), eq("User-Agent")); + verify(response).setHeader(eq("Access-Control-Allow-Origin"), eq("*")); + verify(response, times(0)).setContentLength(anyInt()); + verify(response, times(0)).setContentType(anyString()); + } + + /** + * A Pipeline with the Sequence Element, which adds a new session id to a + * request that has none, an element that accepts the User-Agent header, + * and the JSON builder. + */ + private static Pipeline webPipeline() throws Exception { + return new PipelineBuilder(LoggerFactory.getILoggerFactory()) + .addFlowElement(new SequenceElementBuilder( + LoggerFactory.getILoggerFactory()).build()) + .addFlowElement(new UserAgentElement()) + .addFlowElement(new JsonBuilderElementBuilder( + LoggerFactory.getILoggerFactory()).build()) + .setAutoCloseElements(true) + .build(); + } + + /** + * A GET request with a User-Agent header and no session id. + */ + private static HttpServletRequest webRequest(String userAgent) { + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getMethod()).thenReturn("GET"); + when(request.getRequestURI()).thenReturn("/51dpipeline/json"); + when(request.getHeaderNames()).thenReturn( + Collections.enumeration(Arrays.asList("User-Agent", "X-Test"))); + when(request.getHeader("User-Agent")).thenReturn(userAgent); + when(request.getHeader("X-Test")).thenReturn("not accepted"); + when(request.getRemoteAddr()).thenReturn("1.2.3.4"); + when(request.getScheme()).thenReturn("https"); + return request; + } + + /** + * Add the request's Evidence to the FlowData as the web integration does, + * process it, and return the ETag the JSON endpoint sends. + */ + private static String servedETag( + Pipeline webPipeline, + FlowData data, + HttpServletRequest request) throws IOException { + new WebRequestEvidenceServiceCore.Default() + .addEvidenceFromRequest(data, request); + data.process(); + assertTrue("Processing recorded errors: " + data.getErrors(), + data.getErrors().isEmpty()); + + FlowDataProviderCore provider = mock(FlowDataProviderCore.class); + when(provider.getFlowData(request)).thenReturn(data); + HttpServletResponse served = mock(HttpServletResponse.class); + when(served.getWriter()).thenReturn(new PrintWriter(new StringWriter())); + new ClientsidePropertyServiceCore.Default(provider, webPipeline) + .serveJson(request, served); + ArgumentCaptor etag = ArgumentCaptor.forClass(String.class); + verify(served).setHeader(eq("ETag"), etag.capture()); + return etag.getValue(); + } + + /** + * An element that accepts the User-Agent header and adds nothing, in + * place of an engine. + */ + private static class UserAgentElement + extends FlowElementBase { + + private final EvidenceKeyFilter filter = new EvidenceKeyFilterWhitelist( + Collections.singletonList("header.user-agent"), + String.CASE_INSENSITIVE_ORDER); + + UserAgentElement() { + super(LoggerFactory.getLogger(UserAgentElement.class), null); + } + + @Override + protected void processInternal(FlowData data) { + } + + @Override + public String getElementDataKey() { + return "user-agent-test"; + } + + @Override + public EvidenceKeyFilter getEvidenceKeyFilter() { + return filter; + } + + @Override + public List getProperties() { + return new ArrayList<>(); + } + + @Override + protected void managedResourcesCleanup() { + } + + @Override + protected void unmanagedResourcesCleanup() { + } + } + private void recordProcessingError() { when(flowData.getErrors()).thenReturn( Collections.singletonList(mock(FlowError.class))); diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/UsageSharingEvidenceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/UsageSharingEvidenceTests.java index b06d5e8b2..63e607e38 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/UsageSharingEvidenceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/UsageSharingEvidenceTests.java @@ -109,6 +109,9 @@ private static HttpServletRequest request(Map headers) { for (Map.Entry header : headers.entrySet()) { when(request.getHeader(header.getKey())) .thenReturn(header.getValue()); + when(request.getHeaders(header.getKey())).thenAnswer( + invocation -> Collections.enumeration( + Collections.singletonList(header.getValue()))); } when(request.getCookies()).thenReturn(new Cookie[]{ new Cookie("51D_Profile", "123456")}); @@ -272,16 +275,13 @@ public void Revalidation_SameETag_Gets304() throws Exception { when(provider.getFlowData(revalidation)) .thenReturn(revalidationData); - assertEquals("The revalidation's ETag differs from the one it sends.", - etag.getValue(), - Integer.toString(revalidationData.generateKey( - pipeline.getEvidenceKeyFilter()).hashCode())); - HttpServletResponse revalidationResponse = response(); service.serveJson(revalidation, revalidationResponse); verify(revalidationResponse).setStatus(304); verify(revalidationResponse, never()).setStatus(200); verify(revalidationResponse, never()).setContentType(anyString()); + // The 304 carries the ETag a 200 would, which is the one sent. + verify(revalidationResponse).setHeader("ETag", etag.getValue()); } } From fe5e7c5509f39d93328970baca554a8a688cb620 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 20:01:07 +0000 Subject: [PATCH 11/15] Send a weak ETag of the supplied Evidence, and evaluate If-None-Match as RFC 9110 says The ETag was the exclusive or of the hash codes of the Evidence after processing, sent as a bare number. A bare number is not an entity tag, which RFC 9110 section 8.8.3 writes in double quotes, and the Sequence Element adds a new session id to every request without one, so the ETag of the JavaScript endpoint changed on every request and no revalidation got a 304. The ETag is now W/"" of the values the request supplied, read from the web request record the web integration keeps, for the keys in the Pipeline's Evidence key filter. Keys are lower cased and sorted, and each key and value is written as its length then its UTF-8 bytes, so the same values give the same ETag in every process. It is weak because the content can change for the same Evidence, as when a data file is updated. If-None-Match is evaluated as section 13.1.2 says. All its field lines form one list, "*" matches, and a list matches when one tag has the same quoted part as the ETag, with or without W/ on either side. A field that is neither "*" nor a list of entity tags does not match. Vary now holds the headers of a parallel group's filter, and Content-Length counts the content's bytes in the response's character encoding rather than its characters, as the specification's header table says. --- .../ClientsidePropertyServiceCore.java | 259 ++++++++++++++++-- .../web/services/WebRequestEvidence.java | 4 +- .../web/ClientsidePropertyServiceTests.java | 14 +- 3 files changed, 243 insertions(+), 34 deletions(-) diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java index 2d89adff6..104afa201 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java @@ -29,15 +29,24 @@ import fiftyone.pipeline.core.exceptions.PipelineConfigurationException; import fiftyone.pipeline.core.flowelements.FlowElement; import fiftyone.pipeline.core.flowelements.Pipeline; +import fiftyone.pipeline.engines.fiftyone.data.ShareUsageEvidence; import fiftyone.pipeline.javascriptbuilder.flowelements.*; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Enumeration; import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.TreeMap; import static fiftyone.pipeline.core.Constants.EVIDENCE_HTTPHEADER_PREFIX; import static fiftyone.pipeline.core.Constants.EVIDENCE_SEPERATOR; @@ -130,6 +139,13 @@ private enum ContentTypes { */ private static final String FAILED_CACHE_CONTROL = "private, no-cache"; + /** + * The digest the ETag holds. Every Java platform provides it. + */ + private static final String ETAG_DIGEST = "SHA-256"; + + private static final char[] HEX = "0123456789abcdef".toCharArray(); + /** * Create a new instance. * @param flowDataProviderCore the provider to the {@link FlowData} for @@ -159,11 +175,11 @@ protected final void init() { } for (EvidenceKeyFilter filter : filters) { - // If the filter is a white list or derived type then - // get all HTTP header evidence keys from white list - // and add them to the headers that could affect the - // generated JavaScript. - if (filter.getClass().equals(EvidenceKeyFilterWhitelist.class)) { + // If the filter is a white list or derived type, such as + // the filter of a parallel group, then get all HTTP header + // evidence keys from white list and add them to the + // headers that could affect the generated JavaScript. + if (filter instanceof EvidenceKeyFilterWhitelist) { EvidenceKeyFilterWhitelist whitelist = (EvidenceKeyFilterWhitelist) filter; for (String key : whitelist.getWhitelist().keySet()) { if (key.startsWith(EVIDENCE_HTTPHEADER_PREFIX + EVIDENCE_SEPERATOR) && @@ -203,9 +219,8 @@ public void serveContent( HttpServletRequest request, HttpServletResponse response, ContentTypes contentType) throws IOException { - // Get the hash code. - FlowData flowData = flowDataProviderCore.getFlowData(request); - int hash = flowData.generateKey(pipeline.getEvidenceKeyFilter()).hashCode(); + FlowData flowData = flowDataProviderCore.getFlowData(request); + String etag = getETag(flowData); // A response built while processing recorded errors may lack the // data of the element that failed, so it must not be revalidated @@ -213,16 +228,16 @@ public void serveContent( Collection errors = flowData.getErrors(); boolean failed = errors != null && errors.isEmpty() == false; - String ifNoneMatch = request.getHeader("If-None-Match"); - if (ifNoneMatch != null && - Integer.toString(hash).equals(ifNoneMatch)) { + if (ifNoneMatchMatches( + request.getHeaders("If-None-Match"), + etag)) { // The content has not changed. RFC 9110 section 13.1.2 has a // GET or HEAD answered with 304, carrying the caching headers // of the 200 it stands for, and any other method with 412. String method = request.getMethod(); if ("GET".equals(method) || "HEAD".equals(method)) { response.setStatus(304); - setCachingHeaders(response, hash, failed); + setCachingHeaders(response, etag, failed); } else { response.setStatus(412); response.setHeader("Access-Control-Allow-Origin", "*"); @@ -266,9 +281,9 @@ public void serveContent( setHeaders( response, - hash, + etag, failed, - content == null ? 0 : content.length(), + content, contentType == ContentTypes.JavaScript ? "x-javascript" : "json"); if (content != null) { @@ -278,27 +293,223 @@ public void serveContent( } + /** + * The ETag of the Evidence the request supplied, a weak entity tag + * holding the SHA-256 digest of the supplied values whose keys are + * in the Pipeline's Evidence key filter. + *

+ * The values are read from the {@link WebRequestEvidence}, which + * holds them as the request supplied them, because Flow Elements add + * Evidence while they process, such as the new session id the + * Sequence Element adds to a request with none. A FlowData whose + * Evidence was added without the web integration has no such + * record, so its Evidence is used instead. + *

+ * Keys are lower cased, as Evidence keys are case insensitive, and + * sorted. Each key and value is written as its length in four bytes, + * then its UTF-8 bytes, so the same values give the same ETag in + * every process. The tag is weak because the content can change for + * the same Evidence, for example when a data file is updated. + * @param flowData the {@link FlowData} of the request + * @return the ETag, such as W/"9bee...dbd3" + */ + private String getETag(FlowData flowData) { + Map evidence = flowData.getEvidence().asKeyMap(); + Object supplied = evidence.get(ShareUsageEvidence.HTTP_REQUEST_KEY); + if (supplied instanceof WebRequestEvidence) { + evidence = (WebRequestEvidence) supplied; + } + + EvidenceKeyFilter filter = pipeline.getEvidenceKeyFilter(); + Map values = new TreeMap<>(); + for (Map.Entry entry : evidence.entrySet()) { + if (entry.getValue() != null && + filter.include(entry.getKey())) { + values.put( + entry.getKey().toLowerCase(Locale.ROOT), + String.valueOf(entry.getValue())); + } + } + + MessageDigest digest; + try { + digest = MessageDigest.getInstance(ETAG_DIGEST); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException(e); + } + for (Map.Entry entry : values.entrySet()) { + addToDigest(digest, entry.getKey()); + addToDigest(digest, entry.getValue()); + } + + byte[] hash = digest.digest(); + StringBuilder etag = new StringBuilder(hash.length * 2 + 4); + etag.append("W/\""); + for (byte b : hash) { + etag.append(HEX[(b >> 4) & 0xF]).append(HEX[b & 0xF]); + } + return etag.append('"').toString(); + } + + /** + * Add the text to the digest as its length in four bytes, big + * endian, then its UTF-8 bytes, so that no two lists of texts give + * the same bytes. + */ + private static void addToDigest(MessageDigest digest, String text) { + byte[] bytes = text.getBytes(StandardCharsets.UTF_8); + digest.update(new byte[] { + (byte) (bytes.length >>> 24), + (byte) (bytes.length >>> 16), + (byte) (bytes.length >>> 8), + (byte) bytes.length }); + digest.update(bytes); + } + + /** + * Whether the If-None-Match field matches the ETag, evaluated as RFC + * 9110 section 13.1.2 says. The field lines are one comma separated + * list (section 5.2). "*" matches. A list of entity tags matches when + * one of them has the same quoted part as the ETag, with or without + * W/ on either, which is the weak comparison of section 8.8.3.2. + * Empty list elements and spaces or tabs around commas are allowed + * (sections 5.6.1.2 and 5.6.3). Any other field, such as a tag then + * something that is not a tag, or two tags with no comma between + * them, does not match. + * @param lines the If-None-Match field lines, or null + * @param etag the ETag of the response + * @return true if the method must not be performed + */ + private static boolean ifNoneMatchMatches( + Enumeration lines, + String etag) { + if (lines == null || lines.hasMoreElements() == false) { + return false; + } + StringBuilder field = new StringBuilder(); + boolean first = true; + while (lines.hasMoreElements()) { + String line = lines.nextElement(); + if (first == false) { + field.append(", "); + } + field.append(line == null ? "" : line); + first = false; + } + String value = field.toString(); + if ("*".equals(trimWhitespace(value))) { + return true; + } + + String opaque = etag.substring(etag.indexOf('"')); + boolean matched = false; + boolean afterComma = true; + int index = 0; + while (index < value.length()) { + char c = value.charAt(index); + if (c == ' ' || c == '\t') { + index++; + } else if (c == ',') { + afterComma = true; + index++; + } else if (afterComma) { + int start = value.startsWith("W/", index) ? + index + 2 : + index; + int end = endOfOpaqueTag(value, start); + if (end < 0) { + return false; + } + if (opaque.equals(value.substring(start, end))) { + matched = true; + } + afterComma = false; + index = end; + } else { + return false; + } + } + return matched; + } + + /** + * The index after the opaque tag starting at start, a double quote, + * any characters an entity tag allows, and a double quote, or -1 if + * there is none (RFC 9110 section 8.8.3). + */ + private static int endOfOpaqueTag(String value, int start) { + if (start >= value.length() || value.charAt(start) != '"') { + return -1; + } + for (int index = start + 1; index < value.length(); index++) { + char c = value.charAt(index); + if (c == '"') { + return index + 1; + } + // etagc is %x21 / %x23-7E / obs-text (%x80-FF). + if ((c == 0x21 || + (c >= 0x23 && c <= 0x7E) || + (c >= 0x80 && c <= 0xFF)) == false) { + return -1; + } + } + return -1; + } + + /** + * The text without leading and trailing spaces and tabs. + */ + private static String trimWhitespace(String text) { + int start = 0; + int end = text.length(); + while (start < end && + (text.charAt(start) == ' ' || text.charAt(start) == '\t')) { + start++; + } + while (end > start && + (text.charAt(end - 1) == ' ' || text.charAt(end - 1) == '\t')) { + end--; + } + return text.substring(start, end); + } + /** * Set various HTTP headers on the JavaScript response. * @param response the {@link HttpServletResponse} to add the response * headers to - * @param hash the hash to use for the ETag + * @param etag the ETag * @param failed true when processing recorded errors, which sends * no-cache in place of max-age, and no ETag - * @param contentLength the length of the returned content. This is used - * for the 'Content-Length' header + * @param content the content, or null if there is none. Its length + * in bytes, in the character encoding the response is + * written with, is the 'Content-Length' header * @param contentType the type of content */ private void setHeaders( HttpServletResponse response, - int hash, + String etag, boolean failed, - int contentLength, + String content, String contentType) { response.setContentType("application/" + contentType); - response.setContentLength(contentLength); + response.setContentLength(content == null ? + 0 : + content.getBytes(characterEncoding(response)).length); response.setStatus(200); - setCachingHeaders(response, hash, failed); + setCachingHeaders(response, etag, failed); + } + + /** + * The character encoding the response's writer uses, which is + * ISO-8859-1 when none has been set, as the servlet specification + * says. + */ + private static Charset characterEncoding( + HttpServletResponse response) { + String name = response.getCharacterEncoding(); + return name == null ? + StandardCharsets.ISO_8859_1 : + Charset.forName(name); } /** @@ -307,20 +518,20 @@ private void setHeaders( * errors, and the CORS header. * @param response the {@link HttpServletResponse} to add the response * headers to - * @param hash the hash to use for the ETag + * @param etag the ETag * @param failed true when processing recorded errors, which sends * no-cache in place of max-age, and no ETag */ private void setCachingHeaders( HttpServletResponse response, - int hash, + String etag, boolean failed) { response.setHeader("Vary", stringJoin(headersAffectingJavaScript, ",")); if (failed) { response.setHeader("Cache-Control", FAILED_CACHE_CONTROL); } else { response.setHeader("Cache-Control", stringJoin(cacheControl, ",")); - response.setHeader("ETag", Integer.toString(hash)); + response.setHeader("ETag", etag); } response.setHeader("Access-Control-Allow-Origin", "*"); } diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/WebRequestEvidence.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/WebRequestEvidence.java index 6206d41e5..ff0152154 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/WebRequestEvidence.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/WebRequestEvidence.java @@ -36,7 +36,9 @@ * for a request, accepted or not, keyed the way Evidence is keyed, such as * header.user-agent. It is added to Evidence as * {@link ShareUsageEvidence#HTTP_REQUEST_KEY} for the usage sharing element, - * which accepts no Evidence and reads the request here instead. + * which accepts no Evidence and reads the request here instead. The + * client-side endpoints make their ETag from it too, because it holds the + * values as the request supplied them, before any Flow Element adds its own. *

* It is a map rather than the HttpServletRequest because the usage sharing * element does not depend on the servlet API, and the values are copied so diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index e2e294d4b..17271338b 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -28,7 +28,6 @@ import org.mockito.ArgumentMatcher; import org.slf4j.LoggerFactory; -import fiftyone.pipeline.core.data.DataKey; import fiftyone.pipeline.core.data.ElementData; import fiftyone.pipeline.core.data.ElementPropertyMetaData; import fiftyone.pipeline.core.data.Evidence; @@ -77,7 +76,6 @@ public class ClientsidePropertyServiceTests { private FlowDataProviderCore flowDataProvider; private Pipeline pipeline; private FlowData flowData; - private DataKey dataKey; private Map evidence; private JavaScriptBuilderElement jsElement; private JsonBuilderElement jsonElement; @@ -131,8 +129,6 @@ public void init() throws IOException { jsonElement = mock(JsonBuilderElement.class); flowData = mock(FlowData.class); - dataKey = mock(DataKey.class); - when(flowData.generateKey(any(EvidenceKeyFilter.class))).thenReturn(dataKey); evidence = new HashMap<>(); Evidence flowEvidence = mock(Evidence.class); when(flowEvidence.asKeyMap()).thenReturn(evidence); @@ -835,16 +831,16 @@ private void JavaScript_VaryHeader( JS_CONTENT.length(), JS_CONTENT_TYPE, 200, - String.valueOf(dataKey.hashCode()), + WEAK_ETAG, expectedVary); } private void ValidateResponse( String expectedContent, - Integer contentLength, + Integer contentLength, String contentType, int expectedStatusCode, - String expectedETag, + String expectedETagPattern, String expectedVary) { if (expectedContent == null) { expectedContent = ""; } @@ -884,8 +880,8 @@ private void ValidateResponse( verify(response, times(0)).setHeader(eq("Vary"), anyString()); } - if (expectedETag != null) { - verify(response).setHeader(eq("ETag"), eq(expectedETag)); + if (expectedETagPattern != null) { + verify(response).setHeader(eq("ETag"), matches(expectedETagPattern)); } else { verify(response, times(0)).setHeader(eq("ETag"), anyString()); From 902290de9897e5bdf8707f171faccc459a9b7b4f Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 20:04:04 +0000 Subject: [PATCH 12/15] Remove generateSuppliedKey, as the ETag reads the web request record The ETag is the SHA-256 digest of the values in the web request record the web integration adds to Evidence on every request, which holds the values as the request supplied them. A DataKey cannot give that digest, because it holds only the exclusive or of the values' hash codes and not the keys, so nothing calls generateSuppliedKey and FlowData no longer copies its Evidence before processing. The test of a revalidation after an element adds a session id built its FlowData without the web integration and read If-None-Match with getHeader. Json_ETag_UnchangedWhenSequenceElementAddsSessionId covers the same case with the real Sequence Element through the web integration. --- .../fiftyone/pipeline/core/data/FlowData.java | 14 --- .../core/flowelements/FlowDataDefault.java | 23 +---- .../web/ClientsidePropertyServiceTests.java | 96 ------------------- 3 files changed, 1 insertion(+), 132 deletions(-) diff --git a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/data/FlowData.java b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/data/FlowData.java index 7e1d9130f..763da2bb3 100644 --- a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/data/FlowData.java +++ b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/data/FlowData.java @@ -276,20 +276,6 @@ T getOrAdd( */ DataKey generateKey(EvidenceKeyFilter filter); - /** - * Generate a {@link DataKey} from the evidence as it was when processing - * started, without the values that Flow Elements added or replaced while - * processing. Before processing starts it is the same as - * {@link #generateKey(EvidenceKeyFilter)}. It is for work that depends on - * the request alone, such as a cache validator. - * @param filter an {@link EvidenceKeyFilter} instance that defines the - * values to include in the generated key - * @return a new {@link DataKey} instance - */ - default DataKey generateSuppliedKey(EvidenceKeyFilter filter) { - return generateKey(filter); - } - /** * Get a filter that will only include the evidence keys that can be used by * the elements within the pipeline that created this flow element. diff --git a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java index 7a19dce0a..f8758e126 100644 --- a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java +++ b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java @@ -60,13 +60,6 @@ class FlowDataDefault implements FlowData { private final AtomicBoolean processed = new AtomicBoolean(false); - /** - * A copy of the evidence taken when processing started, which leaves out - * any value a Flow Element adds or replaces while processing. Null until - * then. - */ - private volatile Map suppliedEvidence; - FlowDataDefault(Logger logger, Pipeline pipeline, Evidence evidence) { this.logger = logger; this.pipeline = (PipelineInternal) pipeline; @@ -93,7 +86,6 @@ public void process() { } else if (pipeline.isClosed()) { throw new IllegalStateException("Pipeline has been closed."); } else { - suppliedEvidence = new HashMap<>(evidence.asKeyMap()); pipeline.process(this); } } @@ -372,20 +364,7 @@ public Map getWhere(PropertyMatcher matcher) { @Override public DataKey generateKey(EvidenceKeyFilter filter) { - return generateKey(filter, evidence.asKeyMap()); - } - - @Override - public DataKey generateSuppliedKey(EvidenceKeyFilter filter) { - Map supplied = suppliedEvidence; - return generateKey( - filter, - supplied != null ? supplied : evidence.asKeyMap()); - } - - private static DataKey generateKey( - EvidenceKeyFilter filter, - Map evidenceMap) { + Map evidenceMap = evidence.asKeyMap(); DataKeyBuilder builder = new DataKeyBuilderDefault(); for (String key : evidenceMap.keySet()) { if (filter.include(key)) { diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index f951def48..17271338b 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -24,7 +24,6 @@ import org.junit.Before; import org.junit.Test; -import org.mockito.AdditionalAnswers; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.slf4j.LoggerFactory; @@ -803,101 +802,6 @@ protected void unmanagedResourcesCleanup() { } } - /** - * The ETag is made from the evidence the request supplied. A Flow Element - * that accepts the evidence it adds, as the Sequence Element does with a - * new session id, would otherwise give every request a different ETag, and - * a revalidation would never get a 304. - */ - @Test - public void JavaScript_ElementAddsEvidence_RevalidationGets304() - throws Exception { - try (Pipeline real = new PipelineBuilder() - .addFlowElement(new SessionIdElement()) - .build()) { - pipeline = mock(Pipeline.class); - when(pipeline.getFlowElements()).thenReturn(real.getFlowElements()); - when(pipeline.getElement(JavaScriptBuilderElement.class)).thenReturn(jsElement); - when(pipeline.getElement(JsonBuilderElement.class)).thenReturn(jsonElement); - when(pipeline.getEvidenceKeyFilter()).thenReturn(real.getEvidenceKeyFilter()); - service = new ClientsidePropertyServiceCore.Default( - flowDataProvider, pipeline); - - ArgumentCaptor eTag = ArgumentCaptor.forClass(String.class); - try (FlowData first = real.createFlowData()) { - first.process(); - FlowData wrapped = wrap(first); - when(flowDataProvider.getFlowData(any(HttpServletRequest.class))) - .thenReturn(wrapped); - service.serveJavascript(request, response); - verify(response).setHeader(eq("ETag"), eTag.capture()); - } - - when(request.getHeader("If-None-Match")).thenReturn(eTag.getValue()); - try (FlowData second = real.createFlowData()) { - second.process(); - FlowData wrapped = wrap(second); - when(flowDataProvider.getFlowData(any(HttpServletRequest.class))) - .thenReturn(wrapped); - service.serveJavascript(request, response); - } - - verify(response).setStatus(304); - } - } - - private FlowData wrap(FlowData data) throws Exception { - FlowData wrapped = mock( - FlowData.class, AdditionalAnswers.delegatesTo(data)); - doReturn(pipeline).when(wrapped).getPipeline(); - doReturn(jsData).when(wrapped).getFromElement(jsElement); - doReturn(jsonData).when(wrapped).getFromElement(jsonElement); - return wrapped; - } - - /** - * Accepts and adds a session id as the Sequence Element does. - */ - private static class SessionIdElement - extends FlowElementBase { - - private static final String SESSION_ID = "query.session-id"; - - SessionIdElement() { - super(LoggerFactory.getLogger(SessionIdElement.class), null); - } - - @Override - protected void processInternal(FlowData data) { - if (data.getEvidence().asKeyMap().containsKey(SESSION_ID) == false) { - data.addEvidence(SESSION_ID, java.util.UUID.randomUUID().toString()); - } - } - - @Override - public String getElementDataKey() { - return "session-id-test"; - } - - @Override - public fiftyone.pipeline.core.data.EvidenceKeyFilter getEvidenceKeyFilter() { - return new EvidenceKeyFilterWhitelist(Arrays.asList(SESSION_ID)); - } - - @Override - public List getProperties() { - return new ArrayList<>(); - } - - @Override - protected void managedResourcesCleanup() { - } - - @Override - protected void unmanagedResourcesCleanup() { - } - } - private void recordProcessingError() { when(flowData.getErrors()).thenReturn( Collections.singletonList(mock(FlowError.class))); From 4e42723bff0ce57014b803ad8bee9719b4010965 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 20:48:00 +0000 Subject: [PATCH 13/15] Make the client-side endpoints' max-age configurable, 1800 seconds by default The web integration specification says the max-age of the client-side endpoints' Cache-Control header SHOULD be configurable, so that a site whose JavaScript gathers values that change, such as a location, can shorten it. It was fixed at 1800 seconds. ClientsidePropertyServiceCore.Default gains setCacheMaxAge, which refuses a negative number. The servlet filter reads it from the clientside-cache-max-age init parameter and fails to start on a value that is not a whole number of seconds, 0 or more. The Spring integration reads it from FiftyOneInterceptorConfig.getClientsideCacheMaxAge, a default method, which FiftyOneInterceptorConfigDefault can set. The default stays 1800 seconds, and a failed response still carries private, no-cache. --- .../mvc/components/FiftyOneInterceptor.java | 2 + .../FiftyOneInterceptorConfig.java | 11 ++++ .../FiftyOneInterceptorConfigDefault.java | 19 +++++++ .../services/ClientsidePropertyService.java | 9 ++++ .../web/mvc/FiftyOneInterceptorTests.java | 25 +++++++++ .../java/fiftyone/pipeline/web/Constants.java | 14 +++++ .../fiftyone/pipeline/web/PipelineFilter.java | 42 +++++++++++++-- .../ClientsidePropertyServiceCore.java | 34 +++++++++--- .../web/ClientsidePropertyServiceTests.java | 47 +++++++++++++++++ .../pipeline/web/PipelineFilterTests.java | 52 +++++++++++++++++++ 10 files changed, 246 insertions(+), 9 deletions(-) diff --git a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/components/FiftyOneInterceptor.java b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/components/FiftyOneInterceptor.java index 309b7669b..6ad6ef04c 100644 --- a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/components/FiftyOneInterceptor.java +++ b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/components/FiftyOneInterceptor.java @@ -113,6 +113,8 @@ public FiftyOneInterceptor( resultService.setPipeline(pipeline); clientsidePropertyService.setPipeline(pipeline); + clientsidePropertyService.setCacheMaxAge( + config.getClientsideCacheMaxAge()); fiftyOneJsService.enable(config.getClientsidePropertiesEnabled()); } diff --git a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfig.java b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfig.java index c2d6ddfbc..70138f160 100644 --- a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfig.java +++ b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfig.java @@ -24,6 +24,8 @@ import fiftyone.pipeline.web.mvc.components.FiftyOneInterceptor; +import static fiftyone.pipeline.web.Constants.DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS; + /** * Configuration class for the {@link FiftyOneInterceptor}. */ @@ -40,4 +42,13 @@ public interface FiftyOneInterceptorConfig { * @return true if client-side properties are enabled */ boolean getClientsidePropertiesEnabled(); + + /** + * Get how many seconds a browser may keep the client-side JavaScript or + * JSON before it asks again, the max-age of their Cache-Control header. + * @return the max-age in seconds, 0 or more + */ + default int getClientsideCacheMaxAge() { + return DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS; + } } diff --git a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefault.java b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefault.java index 4b3172ff2..c95b14cd5 100644 --- a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefault.java +++ b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefault.java @@ -23,6 +23,8 @@ package fiftyone.pipeline.web.mvc.configuration; import javax.servlet.ServletContext; + +import static fiftyone.pipeline.web.Constants.DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS; import org.springframework.beans.factory.annotation.Autowired; /** @@ -34,6 +36,9 @@ public class FiftyOneInterceptorConfigDefault implements FiftyOneInterceptorConf private boolean clientsidePropertiesEnabled; + private int clientsideCacheMaxAge = + DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS; + @Autowired public ServletContext servletContext; @@ -45,6 +50,15 @@ public void setClientsidePropertiesEnabled(boolean clientsidePropertiesEnabled) this.clientsidePropertiesEnabled = clientsidePropertiesEnabled; } + /** + * Set how many seconds a browser may keep the client-side JavaScript or + * JSON before it asks again, the max-age of their Cache-Control header. + * @param seconds the max-age, 0 or more + */ + public void setClientsideCacheMaxAge(int seconds) { + this.clientsideCacheMaxAge = seconds; + } + @Override public String getDataFilePath() { return dataFilePath; @@ -54,4 +68,9 @@ public String getDataFilePath() { public boolean getClientsidePropertiesEnabled() { return clientsidePropertiesEnabled; } + + @Override + public int getClientsideCacheMaxAge() { + return clientsideCacheMaxAge; + } } diff --git a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/services/ClientsidePropertyService.java b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/services/ClientsidePropertyService.java index cc026b3b3..83de0f8f5 100644 --- a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/services/ClientsidePropertyService.java +++ b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/services/ClientsidePropertyService.java @@ -38,6 +38,15 @@ @Service public interface ClientsidePropertyService extends ClientsidePropertyServiceCore { void setPipeline(Pipeline pipeline); + + /** + * Set how many seconds a browser may keep a good response before it + * asks again, the max-age of the Cache-Control header. An + * implementation that sets no Cache-Control header ignores it. + * @param seconds the max-age, 0 or more + */ + default void setCacheMaxAge(int seconds) { + } void serveJavascript( HttpServletRequest request, HttpServletResponse response) throws IOException; diff --git a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/FiftyOneInterceptorTests.java b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/FiftyOneInterceptorTests.java index 077a88b46..40c2ce146 100644 --- a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/FiftyOneInterceptorTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/FiftyOneInterceptorTests.java @@ -97,6 +97,31 @@ public void init() throws IOException { uachService); } + /** + * The interceptor gives the client-side service the max-age its + * configuration sets. + */ + @Test + public void FiftyOneInterceptor_SetsTheCacheMaxAge() { + FiftyOneInterceptorConfig config = + mock(FiftyOneInterceptorConfig.class); + when(config.getDataFilePath()) + .thenReturn("src/test/resources/Test.xml"); + when(config.getClientsideCacheMaxAge()).thenReturn(60); + ClientsidePropertyService clientsidePropertyService = + mock(ClientsidePropertyService.class); + + new FiftyOneInterceptor( + config, + mock(PipelineResultService.class), + mock(FlowDataProvider.class), + clientsidePropertyService, + mock(FiftyOneJSService.class), + mock(UACHService.class)); + + verify(clientsidePropertyService, times(1)).setCacheMaxAge(60); + } + /** * Check that 'resultService.process()' is called during * interceptor's preHandle process diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/Constants.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/Constants.java index acd9791db..f43aa381a 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/Constants.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/Constants.java @@ -74,4 +74,18 @@ public class Constants { */ public static final boolean DEFAULT_CLIENTSIDE_ENABLED = true; + /** + * Default number of seconds a browser may keep the client-side + * JavaScript or JSON before it asks again, the max-age of their + * Cache-Control header. + */ + public static final int DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS = 1800; + + /** + * Name of the filter init parameter that sets the max-age of the + * client-side endpoints' Cache-Control header, in seconds. + */ + public static final String INIT_PARAM_CLIENTSIDE_CACHE_MAX_AGE = + "clientside-cache-max-age"; + } diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/PipelineFilter.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/PipelineFilter.java index 098da0926..940669c39 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/PipelineFilter.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/PipelineFilter.java @@ -40,8 +40,10 @@ import java.io.File; import java.io.IOException; +import static fiftyone.pipeline.web.Constants.DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS; import static fiftyone.pipeline.web.Constants.DEFAULT_CLIENTSIDE_ENABLED; import static fiftyone.pipeline.web.Constants.DEFAULT_CONFIG_FILE; +import static fiftyone.pipeline.web.Constants.INIT_PARAM_CLIENTSIDE_CACHE_MAX_AGE; /** * Servlet filter used to intercept HTTP requests and process them using the @@ -80,6 +82,7 @@ public void init(FilterConfig filterConfig) throws ServletException { else { clientsideEnabled = Boolean.parseBoolean(clientsideEnabledString); } + int cacheMaxAgeSeconds = getCacheMaxAgeSeconds(filterConfig); ServletContext context = config.getServletContext(); @@ -105,14 +108,47 @@ public void init(FilterConfig filterConfig) throws ServletException { resultService = new PipelineResultServiceCore.Default(evidenceService, pipeline); flowDataProviderCore = new FlowDataProviderCore.Default(); uachServiceCore = new UACHServiceCore.Default(); - clientsidePropertyServiceCore = new ClientsidePropertyServiceCore.Default( - flowDataProviderCore, - pipeline); + ClientsidePropertyServiceCore.Default clientside = + new ClientsidePropertyServiceCore.Default( + flowDataProviderCore, + pipeline); + clientside.setCacheMaxAge(cacheMaxAgeSeconds); + clientsidePropertyServiceCore = clientside; jsService = new FiftyOneJSServiceCore.Default( clientsidePropertyServiceCore, clientsideEnabled); } + /** + * The max-age of the client-side endpoints' Cache-Control header, in + * seconds, from the filter's init parameter, or the default when the + * parameter is not set. + * @param filterConfig the filter's configuration + * @return the max-age in seconds + * @throws ServletException if the parameter is not a whole number of + * seconds, 0 or more + */ + static int getCacheMaxAgeSeconds(FilterConfig filterConfig) + throws ServletException { + String value = filterConfig.getInitParameter( + INIT_PARAM_CLIENTSIDE_CACHE_MAX_AGE); + if (value == null || value.trim().isEmpty()) { + return DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS; + } + try { + int seconds = Integer.parseInt(value.trim()); + if (seconds >= 0) { + return seconds; + } + } catch (NumberFormatException e) { + // Reported below, as a negative number is. + } + throw new ServletException("The '" + + INIT_PARAM_CLIENTSIDE_CACHE_MAX_AGE + "' init parameter must " + + "be a whole number of seconds, 0 or more, and was '" + value + + "'."); + } + @Override public void doFilter( ServletRequest request, diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java index 104afa201..c5f9561ca 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java @@ -40,7 +40,6 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Enumeration; import java.util.List; @@ -50,6 +49,7 @@ import static fiftyone.pipeline.core.Constants.EVIDENCE_HTTPHEADER_PREFIX; import static fiftyone.pipeline.core.Constants.EVIDENCE_SEPERATOR; +import static fiftyone.pipeline.web.Constants.DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS; import fiftyone.pipeline.javascriptbuilder.data.JavaScriptBuilderData; import fiftyone.pipeline.jsonbuilder.data.JsonBuilderData; import fiftyone.pipeline.jsonbuilder.flowelements.JsonBuilderElement; @@ -126,11 +126,11 @@ private enum ContentTypes { private List headersAffectingJavaScript; /** - * The cache control values that will be set for the JavaScript. + * The cache control value for a good response, which only the + * browser may keep, for the configured max-age. */ - private final List cacheControl = Arrays.asList( - "private", - "max-age=1800"); + private volatile String cacheControl = + cacheControl(DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS); /** * The cache control value for a response built while processing @@ -194,6 +194,28 @@ protected final void init() { } } + /** + * Set how many seconds a browser may keep a good response before it + * asks again, the max-age of the Cache-Control header. The default + * is {@link fiftyone.pipeline.web.Constants#DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS}. + * A shorter time suits JavaScript that gathers values which change, + * such as a location. + * @param seconds the max-age, 0 or more + * @throws IllegalArgumentException if seconds is negative + */ + public void setCacheMaxAge(int seconds) { + if (seconds < 0) { + throw new IllegalArgumentException( + "The max-age of the client-side responses cannot be " + + "negative, and was " + seconds + "."); + } + cacheControl = cacheControl(seconds); + } + + private static String cacheControl(int maxAgeSeconds) { + return "private,max-age=" + maxAgeSeconds; + } + private boolean hasControlChar(String str) { for (char chr : str.toCharArray()) { if(chr <= 31) return true; @@ -530,7 +552,7 @@ private void setCachingHeaders( if (failed) { response.setHeader("Cache-Control", FAILED_CACHE_CONTROL); } else { - response.setHeader("Cache-Control", stringJoin(cacheControl, ",")); + response.setHeader("Cache-Control", cacheControl); response.setHeader("ETag", etag); } response.setHeader("Access-Control-Allow-Origin", "*"); diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index 17271338b..b094684e1 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -514,6 +514,53 @@ public void Json_Post_200() throws IOException { assertEquals(JSON_CONTENT, responseString.toString()); } + /** + * The max-age can be set, as the specification says it SHOULD be, and a + * 200 and a 304 both carry it. + */ + @Test + public void JavaScript_CacheMaxAge_Configured() throws IOException { + Configure(null); + ((ClientsidePropertyServiceCore.Default) service).setCacheMaxAge(60); + + service.serveJavascript(request, response); + + verify(response).setHeader(eq("Cache-Control"), eq("private,max-age=60")); + } + + @Test + public void Json_CacheMaxAge_Configured_304() throws IOException { + Configure(null); + ((ClientsidePropertyServiceCore.Default) service).setCacheMaxAge(0); + ifNoneMatch(etagSent()); + + service.serveJson(request, response); + + verify(response).setStatus(eq(304)); + verify(response).setHeader(eq("Cache-Control"), eq("private,max-age=0")); + } + + /** + * A failed response is not kept, whatever the max-age. + */ + @Test + public void JavaScript_CacheMaxAge_FailedResponseNoCache() + throws IOException { + recordProcessingError(); + Configure(null); + ((ClientsidePropertyServiceCore.Default) service).setCacheMaxAge(60); + + service.serveJavascript(request, response); + + ValidateFailedResponse(JS_CONTENT, JS_CONTENT_TYPE); + } + + @Test(expected = IllegalArgumentException.class) + public void CacheMaxAge_Negative_Refused() { + Configure(null); + ((ClientsidePropertyServiceCore.Default) service).setCacheMaxAge(-1); + } + /** * "*" matches, as the endpoint always has a representation. RFC 9110 * section 13.1.2 step 1. diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/PipelineFilterTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/PipelineFilterTests.java index e0f7b7508..aa11488f4 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/PipelineFilterTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/PipelineFilterTests.java @@ -44,6 +44,7 @@ import static fiftyone.pipeline.web.Constants.*; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -127,6 +128,57 @@ public void PipelineFilter_Initialise() throws ServletException { filter.init(config); } + /** + * The client-side max-age is the default when the init parameter is + * not set. + * @throws ServletException + */ + @Test + public void CacheMaxAge_NotSet_IsTheDefault() throws ServletException { + assertEquals( + DEFAULT_CLIENTSIDE_CACHE_MAX_AGE_SECONDS, + PipelineFilter.getCacheMaxAgeSeconds(config)); + } + + @Test + public void CacheMaxAge_Set() throws ServletException { + FilterConfig withMaxAge = mock(FilterConfig.class); + when(withMaxAge.getInitParameter(INIT_PARAM_CLIENTSIDE_CACHE_MAX_AGE)) + .thenReturn(" 60 "); + + assertEquals(60, PipelineFilter.getCacheMaxAgeSeconds(withMaxAge)); + } + + /** + * A max-age that is not a whole number of seconds, 0 or more, fails + * the filter's start rather than being ignored. + */ + @Test + public void CacheMaxAge_Invalid_Refused() { + for (String value : new String[] { "-1", "1.5", "ten" }) { + FilterConfig invalid = mock(FilterConfig.class); + when(invalid.getInitParameter(INIT_PARAM_CLIENTSIDE_CACHE_MAX_AGE)) + .thenReturn(value); + try { + PipelineFilter.getCacheMaxAgeSeconds(invalid); + fail("'" + value + "' was accepted"); + } catch (ServletException e) { + assertTrue(e.getMessage(), e.getMessage().contains( + INIT_PARAM_CLIENTSIDE_CACHE_MAX_AGE)); + } + } + } + + @Test(expected = ServletException.class) + public void PipelineFilter_Initialise_InvalidCacheMaxAge() + throws ServletException { + when(config.getInitParameter(INIT_PARAM_CLIENTSIDE_CACHE_MAX_AGE)) + .thenReturn("-1"); + filter = new PipelineFilter(); + + filter.init(config); + } + /** * Check that the filter can process a request, and that it correctly adds * a processed FlowData to the request. From 013146b230e1d76874843c63995a10e9746ae534 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 00:13:55 +0000 Subject: [PATCH 14/15] Test the missing builder, empty Vary and negative max-age cases JavaScript_MissingBuilder_MatchingIfNoneMatch_Throws and Json_MissingBuilder_MatchingIfNoneMatch_Throws send a matching If-None-Match to a Pipeline without the builder for the endpoint, and expect a PipelineConfigurationException rather than a 304. JavaScript_VaryHeader_PSEUDO now expects no Vary header where the Evidence key filter names no real header, rather than an empty one, as .NET sets Vary only when the list is not empty. FiftyOneInterceptorConfigDefaultTests expects the Spring setter to refuse a negative max-age and keep 0. The two missing builder tests, JavaScript_VaryHeader_PSEUDO and setClientsideCacheMaxAge_Negative_Throws are written to fail on the previous commit. --- ...FiftyOneInterceptorConfigDefaultTests.java | 60 +++++++++++++++++++ .../web/ClientsidePropertyServiceTests.java | 50 +++++++++++++++- 2 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefaultTests.java diff --git a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefaultTests.java b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefaultTests.java new file mode 100644 index 000000000..6e834cbb3 --- /dev/null +++ b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefaultTests.java @@ -0,0 +1,60 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package fiftyone.pipeline.web.mvc.configuration; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class FiftyOneInterceptorConfigDefaultTests { + + /** + * A negative max-age is refused where it is set, rather than when the + * interceptor starts. + */ + @Test + public void setClientsideCacheMaxAge_Negative_Throws() { + FiftyOneInterceptorConfigDefault config = + new FiftyOneInterceptorConfigDefault(); + try { + config.setClientsideCacheMaxAge(-1); + fail("An IllegalArgumentException should have been thrown"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage(), e.getMessage().contains("-1")); + } + } + + /** + * A max-age of 0 is kept, as it tells the browser to ask again on each + * use. + */ + @Test + public void setClientsideCacheMaxAge_Zero_Kept() { + FiftyOneInterceptorConfigDefault config = + new FiftyOneInterceptorConfigDefault(); + config.setClientsideCacheMaxAge(0); + assertEquals(0, config.getClientsideCacheMaxAge()); + } +} diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java index b094684e1..543e54435 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/ClientsidePropertyServiceTests.java @@ -36,6 +36,7 @@ import fiftyone.pipeline.core.data.EvidenceKeyFilterWhitelist; import fiftyone.pipeline.core.data.FlowData; import fiftyone.pipeline.core.data.FlowError; +import fiftyone.pipeline.core.exceptions.PipelineConfigurationException; import fiftyone.pipeline.core.flowelements.FlowElement; import fiftyone.pipeline.core.flowelements.FlowElementBase; import fiftyone.pipeline.core.flowelements.Pipeline; @@ -54,6 +55,7 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -170,6 +172,10 @@ public void JavaScript_VaryHeader_UA_AND_SEC() throws IOException { JavaScript_VaryHeader(elements, "User-Agent,Sec-ch-ua"); } + /** + * An Evidence key filter that names no real header gives no Vary header, + * rather than an empty one. + */ @Test public void JavaScript_VaryHeader_PSEUDO() throws IOException { List> elements = new ArrayList>(); @@ -177,7 +183,7 @@ public void JavaScript_VaryHeader_PSEUDO() throws IOException { "header.sec-ch-uasec-ch-ua-full-version"); elements.add(element); - JavaScript_VaryHeader(elements, ""); + JavaScript_VaryHeader(elements, null); } @Test @@ -454,6 +460,48 @@ public void Json_MatchingIfNoneMatch_Get_304_CarriesHeaders() Validate304(etag); } + /** + * A Pipeline without the JavaScript builder fails a request for the + * JavaScript with a configuration error even when its If-None-Match + * matches, rather than answering 304. + */ + @Test + public void JavaScript_MissingBuilder_MatchingIfNoneMatch_Throws() + throws IOException { + Configure(ConfigureElements( + Collections.singletonList(Arrays.asList("header.User-Agent")))); + String etag = etagSent(); + ifNoneMatch(etag); + when(pipeline.getElement(JavaScriptBuilderElement.class)).thenReturn(null); + + try { + service.serveJavascript(request, response); + fail("A PipelineConfigurationException should have been thrown"); + } catch (PipelineConfigurationException e) { + verify(response, times(0)).setStatus(eq(304)); + } + } + + /** + * As above, for the JSON endpoint without the JSON builder. + */ + @Test + public void Json_MissingBuilder_MatchingIfNoneMatch_Throws() + throws IOException { + Configure(ConfigureElements( + Collections.singletonList(Arrays.asList("header.User-Agent")))); + String etag = etagSent(); + ifNoneMatch(etag); + when(pipeline.getElement(JsonBuilderElement.class)).thenReturn(null); + + try { + service.serveJson(request, response); + fail("A PipelineConfigurationException should have been thrown"); + } catch (PipelineConfigurationException e) { + verify(response, times(0)).setStatus(eq(304)); + } + } + @Test public void JavaScript_MatchingIfNoneMatch_Head_304() throws IOException { Configure(null); From 7a2686b5a015b189a121aa3963682583f7213e74 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 00:14:43 +0000 Subject: [PATCH 15/15] Check the builder before a 304, leave out an empty Vary, and check the Spring max-age serveContent now finds the builder element for the endpoint before it tests If-None-Match, so a Pipeline without the JavaScriptBuilderElement or JsonBuilderElement fails with a PipelineConfigurationException on every request, rather than answering 304 to a client that already holds an ETag. setCachingHeaders sets Vary only when some header affects the response, as .NET's SetHeaders does, rather than sending an empty Vary header. FiftyOneInterceptorConfigDefault.setClientsideCacheMaxAge refuses a negative max-age with the message the service gives, so the error points at the setting where it is typed. --- .../FiftyOneInterceptorConfigDefault.java | 6 ++ .../ClientsidePropertyServiceCore.java | 62 ++++++++++++------- 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefault.java b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefault.java index c95b14cd5..4fb07181c 100644 --- a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefault.java +++ b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/configuration/FiftyOneInterceptorConfigDefault.java @@ -54,8 +54,14 @@ public void setClientsidePropertiesEnabled(boolean clientsidePropertiesEnabled) * Set how many seconds a browser may keep the client-side JavaScript or * JSON before it asks again, the max-age of their Cache-Control header. * @param seconds the max-age, 0 or more + * @throws IllegalArgumentException if seconds is negative */ public void setClientsideCacheMaxAge(int seconds) { + if (seconds < 0) { + throw new IllegalArgumentException( + "The max-age of the client-side responses cannot be " + + "negative, and was " + seconds + "."); + } this.clientsideCacheMaxAge = seconds; } diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java index c5f9561ca..d3adae4b5 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/ClientsidePropertyServiceCore.java @@ -250,6 +250,41 @@ public void serveContent( Collection errors = flowData.getErrors(); boolean failed = errors != null && errors.isEmpty() == false; + // A Pipeline without the builder for this endpoint is a + // configuration error, so it is reported before a matching + // If-None-Match could answer 304. + JavaScriptBuilderElement jsElement = null; + JsonBuilderElement jsonElement = null; + switch (contentType) { + case JavaScript: + jsElement = flowData.getPipeline().getElement(JavaScriptBuilderElement.class); + if (jsElement == null) { + throw new PipelineConfigurationException( + "Client-side JavaScript has been requested from " + + "the Pipeline. However, the JavaScriptBuilderElement " + + "is not present. To resolve this error, " + + "either disable client-side evidence or ensure " + + "the JavaScriptBuilderElement is added to " + + "your Pipeline."); + } + break; + case Json: + jsonElement = flowData.getPipeline().getElement(JsonBuilderElement.class); + if (jsonElement == null) { + throw new PipelineConfigurationException( + "JSON data has been requested from the Pipeline. " + + "However, the JsonBuilderElement is not " + + "present. To resolve this error, either " + + "disable client-side evidence or ensure " + + "the JsonBuilderElement is added to your " + + "Pipeline." + ); + } + break; + default: + break; + } + if (ifNoneMatchMatches( request.getHeaders("If-None-Match"), etag)) { @@ -269,31 +304,10 @@ public void serveContent( String content = null; switch (contentType) { case JavaScript: - JavaScriptBuilderElement jsElement = flowData.getPipeline().getElement(JavaScriptBuilderElement.class); - if (jsElement == null) { - throw new PipelineConfigurationException( - "Client-side JavaScript has been requested from " + - "the Pipeline. However, the JavaScriptBuilderElement " + - "is not present. To resolve this error, " + - "either disable client-side evidence or ensure " + - "the JavaScriptBuilderElement is added to " + - "your Pipeline."); - } JavaScriptBuilderData jsData = flowData.getFromElement(jsElement); content = jsData == null ? null : jsData.getJavaScript(); break; case Json: - JsonBuilderElement jsonElement = flowData.getPipeline().getElement(JsonBuilderElement.class); - if (jsonElement == null) { - throw new PipelineConfigurationException( - "JSON data has been requested from the Pipeline. " + - "However, the JsonBuilderElement is not " + - "present. To resolve this error, either " + - "disable client-side evidence or ensure " + - "the JsonBuilderElement is added to your " + - "Pipeline." - ); - } JsonBuilderData jsonData = flowData.getFromElement(jsonElement); content = jsonData == null ? null : jsonData.getJson(); break; @@ -548,7 +562,11 @@ private void setCachingHeaders( HttpServletResponse response, String etag, boolean failed) { - response.setHeader("Vary", stringJoin(headersAffectingJavaScript, ",")); + // Where no header affects the response there is no Vary header, + // rather than an empty one. + if (headersAffectingJavaScript.isEmpty() == false) { + response.setHeader("Vary", stringJoin(headersAffectingJavaScript, ",")); + } if (failed) { response.setHeader("Cache-Control", FAILED_CACHE_CONTROL); } else {