[grid] Ensuring response headers are spec compliant. There should be Content-Type and Cache-Control headers. Adding a filter that sets these headers in Router if the remote end did not set them.

Author: barancev

java/server/src/org/openqa/selenium/grid/server/NetworkOptions.java

@@ -22,6 +22,7 @@
 import org.openqa.selenium.grid.config.Config;
 import org.openqa.selenium.grid.web.CheckContentTypeHeader;
 import org.openqa.selenium.grid.web.CheckOriginHeader;
+import org.openqa.selenium.grid.web.EnsureSpecCompliantResponseHeaders;
 import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.remote.http.Filter;
 import org.openqa.selenium.remote.http.HttpClient;
@@ -52,6 +53,8 @@ public Filter getSpecComplianceChecks() {
     // Base case: we do nothing
     Filter toReturn = httpHandler -> httpHandler;
 
+    toReturn = toReturn.andThen(new EnsureSpecCompliantResponseHeaders());
+
     if (config.getBool(NETWORK_SECTION, "relax-checks").orElse(false)) {
       return toReturn;
     }

java/server/src/org/openqa/selenium/grid/web/EnsureSpecCompliantHeaders.java

@@ -32,7 +32,9 @@ public EnsureSpecCompliantHeaders(Collection<String> allowedOriginHosts, Set<Str
     Require.nonNull("Allowed origins list", allowedOriginHosts);
     Require.nonNull("URLs to skip checks on", skipChecksOn);
 
-    filter = new CheckOriginHeader(allowedOriginHosts, skipChecksOn).andThen(new CheckContentTypeHeader(skipChecksOn));
+    filter = new CheckOriginHeader(allowedOriginHosts, skipChecksOn)
+      .andThen(new CheckContentTypeHeader(skipChecksOn))
+      .andThen(new EnsureSpecCompliantResponseHeaders());
   }
 
   @Override

java/server/src/org/openqa/selenium/grid/web/EnsureSpecCompliantResponseHeaders.java

@@ -0,0 +1,52 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.grid.web;
+
+import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
+import static org.openqa.selenium.json.Json.JSON_UTF_8;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.net.MediaType;
+
+import org.openqa.selenium.internal.Require;
+import org.openqa.selenium.remote.http.Contents;
+import org.openqa.selenium.remote.http.Filter;
+import org.openqa.selenium.remote.http.HttpHandler;
+import org.openqa.selenium.remote.http.HttpResponse;
+
+import java.util.Set;
+
+public class EnsureSpecCompliantResponseHeaders implements Filter {
+
+  @Override
+  public HttpHandler apply(HttpHandler httpHandler) {
+    Require.nonNull("Next handler", httpHandler);
+
+    return req -> {
+      HttpResponse res = httpHandler.execute(req);
+      if (res.getHeader("Content-Type") == null) {
+        res.addHeader("Content-Type", JSON_UTF_8);
+      }
+      if (res.getHeader("Cache-Control") == null) {
+        res.addHeader("Cache-Control", "no-cache");
+      }
+      return res;
+    };
+  }
+}

java/server/test/org/openqa/selenium/grid/router/EndToEndTest.java

@@ -105,21 +105,23 @@ public static Collection<Supplier<TestData>> buildGrids() {
   private TearDownFixture[] fixtures;
 
   private HttpClient.Factory clientFactory;
+  private HttpClient client;
 
   @Before
   public void setFields() {
     TestData data = values.get();
     this.server = data.server;
     this.fixtures = data.fixtures;
     this.clientFactory = HttpClient.Factory.createDefault();
+    this.client = clientFactory.createClient(server.getUrl());
   }
 
   @After
   public void stopServers() {
+    Safely.safelyCall(client::close);
     Safely.safelyCall(this.fixtures);
   }
 
-
   private static TestData createStandalone() {
     StringBuilder rawCaps = new StringBuilder();
     try (JsonOutput out = new Json().newOutput(rawCaps)) {
@@ -325,6 +327,8 @@ private static void waitUntilReady(Server<?> server, Duration duration, boolean
         return Boolean.TRUE.equals(status != null &&
                                    Boolean.parseBoolean(status.get("ready").toString()));
       });
+
+    Safely.safelyCall(client::close);
   }
 
   private static void waitUntilReady(Server<?> server, Duration duration) {
@@ -414,7 +418,6 @@ public void shouldAllowPassthroughForW3CMode() {
             "capabilities", ImmutableMap.of(
                 "alwaysMatch", ImmutableMap.of("browserName", "cheese")))));
 
-    HttpClient client = clientFactory.createClient(server.getUrl());
     HttpResponse response = client.execute(request);
 
     assertEquals(200, response.getStatus());
@@ -452,7 +455,6 @@ public void shouldAllowPassthroughForJWPMode() {
             "desiredCapabilities", ImmutableMap.of(
                 "browserName", "cheese"))));
 
-    HttpClient client = clientFactory.createClient(server.getUrl());
     HttpResponse response = client.execute(request);
 
     assertEquals(200, response.getStatus());
@@ -479,6 +481,18 @@ public void shouldDoProtocolTranslationFromJWPLocalEndToW3CRemoteEnd() {
 
   }
 
+  @Test
+  public void responseShouldHaveContentTypeAndCacheControlHeaders() {
+    HttpClient client = HttpClient.Factory.createDefault().createClient(server.getUrl());
+
+    HttpResponse response = client.execute(new HttpRequest(GET, "/status"));
+
+    assertThat(response.getHeader("Content-Type"))
+      .isEqualTo("application/json; charset=utf-8");
+    assertThat(response.getHeader("Cache-Control"))
+      .isEqualTo("no-cache");
+  }
+
   private static class TestData {
     public final Server<?> server;
     public final TearDownFixture[] fixtures;