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..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 @@ -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,21 @@ 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 + * @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; + } + @Override public String getDataFilePath() { return dataFilePath; @@ -54,4 +74,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.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/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 67439d7cc..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 @@ -25,20 +25,31 @@ 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; +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; +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; @@ -115,11 +126,25 @@ 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 + * 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"; + + /** + * 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. @@ -150,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) && @@ -169,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; @@ -194,45 +241,73 @@ 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(); - - 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); + 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 + // into a 304 once processing works again. + 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)) { + // 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, etag, failed); + } else { + response.setStatus(412); + response.setHeader("Access-Control-Allow-Origin", "*"); + } } else { // Otherwise, return the requested content to the client. 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; @@ -242,35 +317,262 @@ public void serveContent( setHeaders( response, - hash, - content == null ? 0 : content.length(), + etag, + failed, + content, contentType == ContentTypes.JavaScript ? "x-javascript" : "json"); - response.getWriter().write(content); + if (content != null) { + response.getWriter().write(content); + } + } + + } + + /** + * 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 contentLength the length of the returned content. This is used - * for the 'Content-Length' header + * @param etag the ETag + * @param failed true when processing recorded errors, which sends + * no-cache in place of max-age, and no ETag + * @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, - int contentLength, + String etag, + boolean failed, + String content, String contentType) { response.setContentType("application/" + contentType); - response.setContentLength(contentLength); + response.setContentLength(content == null ? + 0 : + content.getBytes(characterEncoding(response)).length); response.setStatus(200); - response.setHeader("Cache-Control", stringJoin(cacheControl, ",")); - response.setHeader("Vary", stringJoin(headersAffectingJavaScript, ",")); - response.setHeader("ETag", Integer.toString(hash)); + 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); + } + + /** + * 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 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, + String etag, + boolean failed) { + // 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 { + 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/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 f819bd994..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 @@ -24,23 +24,38 @@ 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.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.exceptions.PipelineConfigurationException; 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.junit.Assert.fail; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -49,7 +64,10 @@ import java.io.StringWriter; 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; @@ -60,7 +78,7 @@ public class ClientsidePropertyServiceTests { private FlowDataProviderCore flowDataProvider; private Pipeline pipeline; private FlowData flowData; - private DataKey dataKey; + private Map evidence; private JavaScriptBuilderElement jsElement; private JsonBuilderElement jsonElement; private JavaScriptBuilderData jsData; @@ -75,9 +93,35 @@ 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); + when(request.getMethod()).thenReturn("GET"); response = mock(HttpServletResponse.class); responseString = new StringWriter(); PrintWriter responseWriter = new PrintWriter(responseString); @@ -87,8 +131,10 @@ 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); + when(flowData.getEvidence()).thenReturn(flowEvidence); jsData = mock(JavaScriptBuilderData.class); when(jsData.getJavaScript()).thenReturn(JS_CONTENT); when(flowData.getFromElement(jsElement)).thenReturn(jsData); @@ -126,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>(); @@ -133,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 @@ -186,8 +236,686 @@ public void JavaScript_VaryHeader_2Element_UA_AND_SEC() throws IOException { JavaScript_VaryHeader(elements, "User-Agent,Sec-ch-ua"); } + /** + * 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 { + 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(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); + } + } + + /** + * 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. 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 { + Configure(ConfigureElements( + Collections.singletonList(Arrays.asList("header.User-Agent")))); + String etag = etagSent(); + recordProcessingError(); + ifNoneMatch(etag); + + 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 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. + */ + @Test + public void JavaScript_MatchingIfNoneMatch_304_CarriesHeaders() + throws IOException { + Configure(ConfigureElements( + Collections.singletonList(Arrays.asList("header.User-Agent")))); + String etag = etagSent(); + ifNoneMatch(etag); + + service.serveJavascript(request, response); + + 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); + } + + /** + * 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); + String etag = etagSent(); + when(request.getMethod()).thenReturn("HEAD"); + ifNoneMatch(etag); + + service.serveJavascript(request, response); + + verify(response).setStatus(eq(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); + String etag = etagSent(); + when(request.getMethod()).thenReturn("POST"); + ifNoneMatch(etag); + + service.serveJson(request, response); + + verify(response).setStatus(eq(412)); + verify(response, times(0)).setStatus(eq(304)); + verify(response).setHeader(eq("Access-Control-Allow-Origin"), eq("*")); + 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. + */ + @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()); + } + + /** + * 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. + */ + @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. + */ + @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); + } + + /** + * 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))); + } + + 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)); @@ -198,16 +926,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 = ""; } @@ -247,8 +975,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()); 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. 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()); } }