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 61022376d..13b121a7e 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 @@ -122,6 +122,8 @@ public FiftyOneInterceptor( resultService.setPipeline(pipeline); clientsidePropertyService.setPipeline(pipeline); + clientsidePropertyService.setCacheMaxAge( + config.getClientsideCacheMaxAge()); fiftyOneJsService.enable(config.getClientsidePropertiesEnabled()); fiftyOneJsService.setPaths(config.getJavaScriptPath(), jsonPath); } @@ -162,6 +164,14 @@ public boolean preHandle( HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + // A request for the 51Degrees JavaScript or JSON whose If-None-Match + // matches the ETag of its Evidence is answered without processing, + // because that Evidence alone decides the content. Nothing is left + // to close. + if (fiftyOneJsService.serveNotModified( + request, response, resultService)) { + return false; + } resultService.process(request); boolean proceed = false; 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 5dcdf3cd2..712e487b5 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}. */ @@ -41,6 +43,15 @@ public interface FiftyOneInterceptorConfig { */ 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; + } + /** * Get the end of the request path the JavaScript is served for. * @return the path, or null or empty for the default 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 bd22601cf..3bd000b66 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; + private String javaScriptPath; private String jsonPath; @@ -53,6 +58,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; + } + public void setJavaScriptPath(String javaScriptPath) { this.javaScriptPath = javaScriptPath; } @@ -98,4 +118,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 3f000fa3b..364db9d32 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 @@ -30,8 +30,14 @@ import static org.mockito.Mockito.*; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.lang.reflect.Field; +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; @@ -53,6 +59,10 @@ import fiftyone.pipeline.web.mvc.services.FiftyOneJSService; import fiftyone.pipeline.web.mvc.services.PipelineResultService; import fiftyone.pipeline.web.mvc.services.UACHService; +import fiftyone.pipeline.web.mvc.services.WebRequestEvidenceService; + +import static fiftyone.pipeline.web.Constants.CORE_JS_NAME; +import static fiftyone.pipeline.web.Constants.HTTPCONTEXT_FLOWDATA; public class FiftyOneInterceptorTests { private HttpServletRequest request; @@ -107,6 +117,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 @@ -149,6 +184,68 @@ public void FiftyOneInterceptor_PreHandle_ServesJavaScript_ClosesFlowData() .close(); } + /** + * A request for the JavaScript whose If-None-Match matches the ETag of + * the Evidence it supplies is answered with a 304 without processing, as + * the servlet filter does. + */ + @Test + public void FiftyOneInterceptor_MatchingIfNoneMatch_304_NotProcessed() + throws Exception { + FiftyOneInterceptorConfig config = + mock(FiftyOneInterceptorConfig.class); + when(config.getDataFilePath()) + .thenReturn("src/test/resources/Test.xml"); + when(config.getClientsidePropertiesEnabled()).thenReturn(true); + when(config.getClientsideCacheMaxAge()).thenReturn(1800); + when(config.getSequenceElementEnabled()).thenReturn(true); + when(config.getSetHeadersElementEnabled()).thenReturn(true); + FlowDataProvider provider = new FlowDataProvider.Default(); + ClientsidePropertyService clientside = + new ClientsidePropertyService.Default(provider); + FiftyOneInterceptor real = new FiftyOneInterceptor( + config, + new PipelineResultService.Default( + new WebRequestEvidenceService.Default()), + provider, + clientside, + new FiftyOneJSService.Default(clientside), + new UACHService.Default(provider)); + + Map attributes = new HashMap<>(); + HttpServletRequest revalidation = mock(HttpServletRequest.class); + when(revalidation.getMethod()).thenReturn("GET"); + when(revalidation.getRequestURI()).thenReturn("/" + CORE_JS_NAME); + when(revalidation.getContextPath()).thenReturn(""); + when(revalidation.getScheme()).thenReturn("https"); + when(revalidation.getHeaderNames()).thenAnswer(invocation -> + Collections.enumeration(Arrays.asList("Host", "If-None-Match"))); + when(revalidation.getHeader("Host")).thenReturn("example.com"); + when(revalidation.getHeader("If-None-Match")).thenReturn("*"); + when(revalidation.getHeaders("If-None-Match")).thenAnswer(invocation -> + Collections.enumeration(Collections.singletonList("*"))); + doAnswer(invocation -> attributes.put( + invocation.getArgument(0), invocation.getArgument(1))) + .when(revalidation).setAttribute(anyString(), any()); + when(revalidation.getAttribute(anyString())).thenAnswer(invocation -> + attributes.get((String) invocation.getArgument(0))); + HttpServletResponse answer = mock(HttpServletResponse.class); + when(answer.getWriter()) + .thenReturn(new PrintWriter(new StringWriter())); + + try { + assertFalse(real.preHandle(revalidation, answer, handler)); + } finally { + real.pipeline.close(); + } + + verify(revalidation, never()).setAttribute( + eq(HTTPCONTEXT_FLOWDATA), any()); + verify(answer).setStatus(304); + verify(answer).setHeader( + eq("Cache-Control"), eq("private,max-age=1800")); + } + private FiftyOneInterceptor newInterceptor( FiftyOneInterceptorConfig config, FiftyOneJSService jsService) throws IOException { 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 7298dd10e..70efca179 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 @@ -81,6 +81,20 @@ 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"; + /** * Default value for the options that add the Sequence Element and the * Set Headers Element. 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 b1090a6ef..92fd7c546 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 @@ -42,9 +42,11 @@ import static fiftyone.pipeline.web.Constants.DEFAULT_JSON_PATH; import static fiftyone.pipeline.web.Constants.CORE_JS_NAME; +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.DEFAULT_ELEMENT_ENABLED; +import static fiftyone.pipeline.web.Constants.INIT_PARAM_CLIENTSIDE_CACHE_MAX_AGE; import static fiftyone.pipeline.web.Constants.INIT_PARAM_JAVASCRIPT_PATH; import static fiftyone.pipeline.web.Constants.INIT_PARAM_JSON_PATH; import static fiftyone.pipeline.web.Constants.INIT_PARAM_SEQUENCE_ELEMENT_ENABLED; @@ -87,6 +89,7 @@ public void init(FilterConfig filterConfig) throws ServletException { else { clientsideEnabled = Boolean.parseBoolean(clientsideEnabledString); } + int cacheMaxAgeSeconds = getCacheMaxAgeSeconds(filterConfig); boolean sequenceElementEnabled = getBooleanInitParameter( filterConfig, @@ -126,9 +129,12 @@ 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, @@ -153,12 +159,52 @@ private static String getPathInitParameter( defaultValue : value.trim(); } + /** + * 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, ServletResponse response, FilterChain chain) throws IOException, ServletException { + // A request for the 51Degrees JavaScript or JSON whose If-None-Match + // matches the ETag of its Evidence is answered without processing, + // because that Evidence alone decides the content. + if (jsService.serveNotModified( + (HttpServletRequest) request, + (HttpServletResponse) response, + resultService)) { + return; + } + // Populate the request properties and store against the // HttpContext. resultService.process((HttpServletRequest)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 d51d2cfe9..596c1c690 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; @@ -82,6 +93,39 @@ void serveJson( HttpServletRequest request, HttpServletResponse response) throws IOException; + /** + * Answer a request for the JavaScript whose If-None-Match matches the + * ETag of the Evidence in the {@link FlowData}, which has not been + * processed. A GET or HEAD gets a 304 carrying the headers of a good + * response, and any other method a 412. + * @param flowData the request's {@link FlowData}, holding its Evidence + * @param request the {@link HttpServletRequest} to answer + * @param response the {@link HttpServletResponse} to write the answer to + * @return true if the request was answered, false if it is to be + * processed + */ + default boolean serveJavascriptNotModified( + FlowData flowData, + HttpServletRequest request, + HttpServletResponse response) { + return false; + } + + /** + * As {@link #serveJavascriptNotModified}, for the JSON. + * @param flowData the request's {@link FlowData}, holding its Evidence + * @param request the {@link HttpServletRequest} to answer + * @param response the {@link HttpServletResponse} to write the answer to + * @return true if the request was answered, false if it is to be + * processed + */ + default boolean serveJsonNotModified( + FlowData flowData, + HttpServletRequest request, + HttpServletResponse response) { + return false; + } + /** * Default implementation of the {@link ClientsidePropertyServiceCore} * interface. @@ -126,11 +170,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 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 final List cacheControl = Arrays.asList( - "private", - "max-age=1800"); + 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. @@ -201,6 +259,28 @@ private List getHeadersAffectingJavaScript() { return headersAffectingJavaScript; } + /** + * 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; @@ -226,45 +306,61 @@ 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 (ifNoneMatchApplies(request, etag)) { + answerMatch(request, response, etag); } 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; @@ -274,35 +370,347 @@ 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); + } } } + @Override + public boolean serveJavascriptNotModified( + FlowData flowData, + HttpServletRequest request, + HttpServletResponse response) { + return serveNotModified( + flowData, request, response, JavaScriptBuilderElement.class); + } + + @Override + public boolean serveJsonNotModified( + FlowData flowData, + HttpServletRequest request, + HttpServletResponse response) { + return serveNotModified( + flowData, request, response, JsonBuilderElement.class); + } + + /** + * Answer the request if its If-None-Match matches the ETag of the + * Evidence in the FlowData. A Pipeline without the builder for the + * endpoint is left to be processed, so that the configuration error + * is reported. + */ + private boolean serveNotModified( + FlowData flowData, + HttpServletRequest request, + HttpServletResponse response, + Class> builder) { + if (pipeline == null || + flowData.getPipeline().getElement(builder) == null) { + return false; + } + String etag = getETag(flowData); + if (ifNoneMatchApplies(request, etag) == false) { + return false; + } + answerMatch(request, response, etag); + return true; + } + + /** + * Answer a request whose If-None-Match matches. RFC 9110 section + * 13.1.2 has a GET or HEAD answered with 304 and any other method + * with 412. The ETag depends on the supplied Evidence alone, so the + * 304 carries the headers of a good response, as the 200 it stands + * for would when processing records no errors. + */ + private void answerMatch( + HttpServletRequest request, + HttpServletResponse response, + String etag) { + String method = request.getMethod(); + if ("GET".equals(method) || "HEAD".equals(method)) { + response.setStatus(304); + setCachingHeaders(response, etag, false); + } else { + response.setStatus(412); + response.setHeader("Access-Control-Allow-Origin", "*"); + } + } + + /** + * 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 request's If-None-Match matches the ETag. RFC 9110 + * section 13.2.1 has a server ignore the conditional headers of a + * method that does not select or change a representation, such as + * CONNECT, OPTIONS or TRACE, so for those it never matches. + * @param request the request + * @param etag the ETag of the response + * @return true if the method must not be performed + */ + private static boolean ifNoneMatchApplies( + HttpServletRequest request, + String etag) { + String method = request.getMethod(); + if ("CONNECT".equals(method) || "OPTIONS".equals(method) || + "TRACE".equals(method)) { + return false; + } + return ifNoneMatchMatches( + request.getHeaders("If-None-Match"), + etag); + } + + /** + * 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(getHeadersAffectingJavaScript(), ",")); - 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. + List vary = getHeadersAffectingJavaScript(); + if (vary.isEmpty() == false) { + response.setHeader("Vary", stringJoin(vary, ",")); + } + 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/FiftyOneJSServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/FiftyOneJSServiceCore.java index 70d27fa0b..7ec017007 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/FiftyOneJSServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/FiftyOneJSServiceCore.java @@ -66,6 +66,29 @@ boolean serveJS(HttpServletRequest request, HttpServletResponse response) boolean serveJson(HttpServletRequest request, HttpServletResponse response) throws IOException; + /** + * Answer a request for the 51Degrees JavaScript or JSON whose + * If-None-Match matches the ETag of the Evidence it supplies, without + * processing it, because that Evidence alone decides the content. A GET + * or HEAD gets a 304 and any other method a 412. Call it before the + * request is processed. + * @param request the {@link HttpServletRequest} to answer + * @param response the {@link HttpServletResponse} to write the answer to + * @param resultService the service that creates the request's + * {@link FlowData}, with the Evidence processing + * would use + * @return true if the request was answered and is not to be processed, + * false otherwise + * @throws IOException if there was a failure reading or writing to the + * request or response + */ + default boolean serveNotModified( + HttpServletRequest request, + HttpServletResponse response, + PipelineResultServiceCore resultService) throws IOException { + return false; + } + /** * Default implementation of the {@link FiftyOneJSServiceCore} service. */ @@ -192,6 +215,38 @@ public boolean serveJson( return result; } + @Override + public boolean serveNotModified( + HttpServletRequest request, + HttpServletResponse response, + PipelineResultServiceCore resultService) throws IOException { + // Most requests carry no If-None-Match, and need nothing more. + if (enabled == false || + request.getHeader("If-None-Match") == null) { + return false; + } + boolean javaScript = matches(request, javaScriptPath); + if (javaScript == false && + matches(request, jsonPath) == false && + matches(request, CORE_JSON_NAME) == false) { + return false; + } + try (FlowData flowData = resultService.createFlowData(request)) { + if (flowData == null) { + return false; + } + return javaScript ? + clientsidePropertyServiceCore.serveJavascriptNotModified( + flowData, request, response) : + clientsidePropertyServiceCore.serveJsonNotModified( + flowData, request, response); + } catch (IOException | RuntimeException e) { + throw e; + } catch (Exception e) { + throw new IOException("FlowData could not be disposed of.", e); + } + } + private void serveCoreJson( HttpServletRequest request, HttpServletResponse response) throws IOException { diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/PipelineResultServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/PipelineResultServiceCore.java index 289c90ff9..c6692a172 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/PipelineResultServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/PipelineResultServiceCore.java @@ -42,6 +42,17 @@ public interface PipelineResultServiceCore { */ void process(HttpServletRequest request); + /** + * Create a FlowData holding the Evidence from the request, as + * {@link #process(HttpServletRequest)} would, without processing it. The + * caller closes it. + * @param request the {@link HttpServletRequest} to take the Evidence from + * @return the FlowData, or null if this service cannot create one + */ + default FlowData createFlowData(HttpServletRequest request) { + return null; + } + /** * Default implementation of the {@link PipelineResultServiceCore} service. */ @@ -79,23 +90,38 @@ public Default( @Override @SuppressWarnings("PMD.FlowDataInTryWithResources") public void process(HttpServletRequest request) { - // Create the flowData - FlowData flowData = pipeline.createFlowData(); + FlowData flowData = createFlowData(request); try { - // Extract the required pieces of evidence from the request - evidenceService.addEvidenceFromRequest(flowData, request); // Start processing the data flowData.process(); } catch (RuntimeException | Error e) { - try { - flowData.close(); - } catch (Exception closeException) { - e.addSuppressed(closeException); - } + closeAfterFailure(flowData, e); throw e; } // Store the FlowData in the request request.setAttribute(HTTPCONTEXT_FLOWDATA, flowData); } + + @Override + @SuppressWarnings("PMD.FlowDataInTryWithResources") + public FlowData createFlowData(HttpServletRequest request) { + FlowData flowData = pipeline.createFlowData(); + try { + // Extract the required pieces of evidence from the request + evidenceService.addEvidenceFromRequest(flowData, request); + } catch (RuntimeException | Error e) { + closeAfterFailure(flowData, e); + throw e; + } + return flowData; + } + + private static void closeAfterFailure(FlowData flowData, Throwable e) { + try { + flowData.close(); + } catch (Exception closeException) { + e.addSuppressed(closeException); + } + } } } 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 31c8f9d46..32a80f259 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,39 @@ 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 +65,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 +79,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 +94,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 +132,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); @@ -148,6 +195,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>(); @@ -155,7 +206,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 @@ -208,8 +259,799 @@ 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); + } + + /** + * The ETag depends on the Evidence alone, so a matching If-None-Match gets + * the same 304 while processing fails as at any other time, carrying the + * headers of a good response. A browser holding a good response keeps it, + * with its ETag and max-age. + */ + @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); + + Validate304(etag); + } + + /** + * 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()); + } + + /** + * RFC 9110 section 13.2.1 has a server ignore If-None-Match on a method + * that does not select or change a representation, such as OPTIONS, so + * a matching one is processed and answered as without the header. + */ + @Test + public void Json_MatchingIfNoneMatch_Options_Ignored() throws IOException { + Configure(null); + String etag = etagSent(); + when(request.getMethod()).thenReturn("OPTIONS"); + ifNoneMatch(etag); + + service.serveJson(request, response); + + verify(response, times(0)).setStatus(eq(412)); + verify(response, times(0)).setStatus(eq(304)); + verify(response).setStatus(eq(200)); + } + + /** + * Before processing, If-None-Match on a TRACE, an OPTIONS or a CONNECT is + * ignored, so the request is left to be processed. + */ + @Test + public void NotModified_IgnoredMethods_NotAnswered() throws IOException { + Configure(null); + for (String method : new String[] {"CONNECT", "OPTIONS", "TRACE"}) { + when(request.getMethod()).thenReturn(method); + ifNoneMatch("*"); + + assertFalse(method, + service.serveJavascriptNotModified(flowData, request, response)); + assertFalse(method, + service.serveJsonNotModified(flowData, request, response)); + verify(response, times(0)).setStatus(anyInt()); + } + } + + /** + * 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); + } + + /** + * Before processing, a matching If-None-Match gets a 304 carrying the + * headers of a good response. + */ + @Test + public void JavaScript_NotModified_Matching_304() throws IOException { + Configure(ConfigureElements( + Collections.singletonList(Arrays.asList("header.User-Agent")))); + String etag = etagSent(); + ifNoneMatch(etag); + + assertTrue( + service.serveJavascriptNotModified(flowData, request, response)); + + Validate304(etag); + } + + /** + * Before processing, a POST whose If-None-Match matches gets a 412. + */ + @Test + public void Json_NotModified_Post_412() throws IOException { + Configure(null); + String etag = etagSent(); + when(request.getMethod()).thenReturn("POST"); + ifNoneMatch(etag); + + assertTrue(service.serveJsonNotModified(flowData, request, response)); + + verify(response).setStatus(eq(412)); + verify(response, times(0)).setHeader(eq("ETag"), anyString()); + assertEquals("", responseString.toString()); + } + + /** + * An If-None-Match that does not match leaves the request to be processed, + * with nothing written. + */ + @Test + public void JavaScript_NotModified_OtherTag_NotAnswered() throws IOException { + Configure(null); + ifNoneMatch("W/\"other\""); + + assertFalse( + service.serveJavascriptNotModified(flowData, request, response)); + + verify(response, times(0)).setStatus(anyInt()); + verify(response, times(0)).setHeader(anyString(), anyString()); + } + + /** + * A Pipeline without the builder for the endpoint is left to be + * processed, which reports the configuration error. + */ + @Test + public void Json_NotModified_MissingBuilder_NotAnswered() { + Configure(null); + ifNoneMatch("*"); + when(pipeline.getElement(JsonBuilderElement.class)).thenReturn(null); + + assertFalse(service.serveJsonNotModified(flowData, request, response)); + + verify(response, times(0)).setStatus(anyInt()); + } + + /** + * A service whose Pipeline is not set yet answers nothing. + */ + @Test + public void NotModified_NoPipeline_NotAnswered() { + service = new ClientsidePropertyServiceCore.Default( + flowDataProvider, null); + ifNoneMatch("*"); + + assertFalse( + service.serveJavascriptNotModified(flowData, request, response)); + verify(response, times(0)).setStatus(anyInt()); + } + + /** + * 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)); @@ -220,16 +1062,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 = ""; } @@ -269,8 +1111,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/FiftyOneJSServiceCoreTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/FiftyOneJSServiceCoreTests.java index c0f8ae388..cf9819fe4 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/FiftyOneJSServiceCoreTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/FiftyOneJSServiceCoreTests.java @@ -22,8 +22,10 @@ package fiftyone.pipeline.web; +import fiftyone.pipeline.core.data.FlowData; import fiftyone.pipeline.web.services.ClientsidePropertyServiceCore; import fiftyone.pipeline.web.services.FiftyOneJSServiceCore; +import fiftyone.pipeline.web.services.PipelineResultServiceCore; import org.junit.Before; import org.junit.Test; @@ -185,4 +187,95 @@ public void ClientSideDisabled_NothingWritten() throws IOException { verify(clientside, never()).serveJavascript(request, response); verify(clientside, never()).serveJson(request, response); } + + // The tests below stub and verify createFlowData on a mock, which + // creates nothing to close, so the FlowData rule does not apply. + + /** + * A request with no If-None-Match is left to be processed, and no + * FlowData is created for it. + */ + @Test + @SuppressWarnings("PMD.FlowDataInTryWithResources") + public void NotModified_NoIfNoneMatch_NothingCreated() throws IOException { + FiftyOneJSServiceCore service = + new FiftyOneJSServiceCore.Default(clientside, true); + PipelineResultServiceCore results = + mock(PipelineResultServiceCore.class); + requestPath("/51Degrees.core.js"); + + assertFalse(service.serveNotModified(request, response, results)); + + verify(results, never()).createFlowData(request); + } + + /** + * A request for another path is left to be processed, whatever its + * If-None-Match. + */ + @Test + @SuppressWarnings("PMD.FlowDataInTryWithResources") + public void NotModified_OtherPath_NothingCreated() throws IOException { + FiftyOneJSServiceCore service = + new FiftyOneJSServiceCore.Default(clientside, true); + PipelineResultServiceCore results = + mock(PipelineResultServiceCore.class); + when(request.getHeader("If-None-Match")).thenReturn("*"); + requestPath("/page"); + + assertFalse(service.serveNotModified(request, response, results)); + + verify(results, never()).createFlowData(request); + } + + /** + * With the client-side features off, nothing is answered early. + */ + @Test + @SuppressWarnings("PMD.FlowDataInTryWithResources") + public void NotModified_ClientSideDisabled_NothingCreated() + throws IOException { + FiftyOneJSServiceCore service = + new FiftyOneJSServiceCore.Default(clientside, false); + PipelineResultServiceCore results = + mock(PipelineResultServiceCore.class); + when(request.getHeader("If-None-Match")).thenReturn("*"); + requestPath("/51Degrees.core.js"); + + assertFalse(service.serveNotModified(request, response, results)); + + verify(results, never()).createFlowData(request); + } + + /** + * A revalidation of the JavaScript or the JSON goes to its own endpoint + * with a FlowData created for it, which is closed afterwards. + */ + @Test + @SuppressWarnings("PMD.FlowDataInTryWithResources") + public void NotModified_JavaScriptAndJson_AskTheirEndpoint() + throws Exception { + FiftyOneJSServiceCore service = + new FiftyOneJSServiceCore.Default(clientside, true); + PipelineResultServiceCore results = + mock(PipelineResultServiceCore.class); + FlowData flowData = mock(FlowData.class); + when(results.createFlowData(request)).thenReturn(flowData); + when(request.getHeader("If-None-Match")).thenReturn("*"); + when(clientside.serveJavascriptNotModified(flowData, request, response)) + .thenReturn(true); + when(clientside.serveJsonNotModified(flowData, request, response)) + .thenReturn(true); + + requestPath("/51Degrees.core.js"); + assertTrue(service.serveNotModified(request, response, results)); + requestPath("/51dpipeline/json"); + assertTrue(service.serveNotModified(request, response, results)); + + verify(clientside, times(1)) + .serveJavascriptNotModified(flowData, request, response); + verify(clientside, times(1)) + .serveJsonNotModified(flowData, request, response); + verify(flowData, times(2)).close(); + } } 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 d0e60d9e4..7369541ac 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 @@ -51,7 +51,11 @@ import javax.servlet.http.HttpServletResponse; import java.io.File; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.lang.reflect.Field; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -148,6 +152,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. @@ -382,4 +437,110 @@ public void StartupHelpers_NoJsonPath_IsTheDefault() throws Exception { } } } + + /** + * A request for the JavaScript whose If-None-Match matches the ETag of + * the Evidence it supplies is answered with a 304 carrying the headers of + * a good response, without processing and without the rest of the chain. + */ + @Test + public void PipelineFilter_MatchingIfNoneMatch_304_NotProcessed() + throws Exception { + filter = new PipelineFilter(); + filter.init(config); + HttpServletRequest revalidation = + clientsideRequest("GET", "/" + CORE_JS_NAME, "*"); + HttpServletResponse answer = clientsideResponse(); + + filter.doFilter(revalidation, answer, chain); + + verify(revalidation, never()).setAttribute( + eq(HTTPCONTEXT_FLOWDATA), any()); + verify(answer).setStatus(304); + verify(answer).setHeader( + eq("Cache-Control"), eq("private,max-age=1800")); + verify(answer).setHeader(eq("ETag"), matches("W/\"[0-9a-f]{64}\"")); + verify(chain, never()).doFilter( + any(ServletRequest.class), any(ServletResponse.class)); + } + + /** + * A POST for the JSON whose If-None-Match matches gets a 412, without + * processing. + */ + @Test + public void PipelineFilter_MatchingIfNoneMatch_Post_412_NotProcessed() + throws Exception { + filter = new PipelineFilter(); + filter.init(config); + HttpServletRequest revalidation = + clientsideRequest("POST", DEFAULT_JSON_PATH, "*"); + HttpServletResponse answer = clientsideResponse(); + + filter.doFilter(revalidation, answer, chain); + + verify(revalidation, never()).setAttribute( + eq(HTTPCONTEXT_FLOWDATA), any()); + verify(answer).setStatus(412); + verify(chain, never()).doFilter( + any(ServletRequest.class), any(ServletResponse.class)); + } + + /** + * A request for the JavaScript whose If-None-Match does not match is + * processed and gets the JavaScript. + */ + @Test + public void PipelineFilter_OtherIfNoneMatch_Processed() throws Exception { + filter = new PipelineFilter(); + filter.init(config); + HttpServletRequest request = + clientsideRequest("GET", "/" + CORE_JS_NAME, "W/\"other\""); + HttpServletResponse answer = clientsideResponse(); + + filter.doFilter(request, answer, chain); + + verify(request, times(1)).setAttribute( + eq(HTTPCONTEXT_FLOWDATA), any(FlowData.class)); + verify(answer).setStatus(200); + verify(answer, never()).setStatus(304); + } + + /** + * A request for the given path with one If-None-Match field line, whose + * attributes are kept as a servlet container keeps them. + */ + private static HttpServletRequest clientsideRequest( + String method, + String path, + String ifNoneMatch) { + Map attributes = new HashMap<>(); + HttpServletRequest clientside = mock(HttpServletRequest.class); + when(clientside.getMethod()).thenReturn(method); + when(clientside.getRequestURI()).thenReturn(path); + when(clientside.getContextPath()).thenReturn(""); + when(clientside.getScheme()).thenReturn("https"); + when(clientside.getLocalAddr()).thenReturn(""); + when(clientside.getRequestURL()) + .thenReturn(new StringBuffer("https://example.com" + path)); + when(clientside.getHeaderNames()).thenAnswer(invocation -> + Collections.enumeration(Arrays.asList("Host", "If-None-Match"))); + when(clientside.getHeader("Host")).thenReturn("example.com"); + when(clientside.getHeader("If-None-Match")).thenReturn(ifNoneMatch); + when(clientside.getHeaders("If-None-Match")).thenAnswer(invocation -> + Collections.enumeration(Collections.singletonList(ifNoneMatch))); + doAnswer(invocation -> attributes.put( + invocation.getArgument(0), invocation.getArgument(1))) + .when(clientside).setAttribute(anyString(), any()); + when(clientside.getAttribute(anyString())).thenAnswer(invocation -> + attributes.get((String) invocation.getArgument(0))); + return clientside; + } + + private static HttpServletResponse clientsideResponse() throws IOException { + HttpServletResponse answer = mock(HttpServletResponse.class); + when(answer.getWriter()) + .thenReturn(new PrintWriter(new StringWriter())); + return answer; + } } 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()); } }