From 79e4135349adb911a4464fb4baae324321a3457c Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 7 May 2019 12:47:25 -0600 Subject: [PATCH 1/6] Added an HTTP-Header equivalent for 'format' TR Query Param --- .../traffic_router/core/http/RouterFilter.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/RouterFilter.java b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/RouterFilter.java index bed96b269d..80200061de 100644 --- a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/RouterFilter.java +++ b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/RouterFilter.java @@ -38,9 +38,11 @@ import java.util.Map; import java.util.Set; +@SuppressWarnings("PMD.AvoidDuplicateLiterals") public class RouterFilter extends OncePerRequestFilter { private static final Logger ACCESS = LogManager.getLogger("org.apache.traffic_control.traffic_router.core.access"); public static final String REDIRECT_QUERY_PARAM = "trred"; + public static final String FORMAT_HEADER = "X-TC-Format"; private static final String HEAD = "HEAD"; @Autowired @@ -161,9 +163,11 @@ private void setMultiResponse(final HTTPRouteResult routeResult, final HttpServl } } + @SuppressWarnings("PMD.CyclomaticComplexity") private void setSingleResponse(final HTTPRouteResult routeResult, final HttpServletRequest httpServletRequest, final HttpServletResponse response, final HTTPAccessRecord.Builder httpAccessRecordBuilder) throws IOException { final String redirect = httpServletRequest.getParameter(REDIRECT_QUERY_PARAM); final String format = httpServletRequest.getParameter("format"); + final String formatHdr = httpServletRequest.getHeader(FORMAT_HEADER); final URL location = routeResult.getUrl(); if (routeResult.getDeliveryService() != null) { @@ -182,6 +186,13 @@ private void setSingleResponse(final HTTPRouteResult routeResult, final HttpServ httpAccessRecordBuilder.responseURLs(routeResult.getUrls()); } + httpAccessRecordBuilder.responseCode(HttpServletResponse.SC_OK); + } else if ("json".equals(formatHdr) || "application/json".equals(formatHdr)) { + response.setContentType("application/json"); + if (!HEAD.equals(httpServletRequest.getMethod())) { + response.getWriter().println(routeResult.toMultiLocationJSONString()); + httpAccessRecordBuilder.responseURLs(routeResult.getUrls()); + } httpAccessRecordBuilder.responseCode(HttpServletResponse.SC_OK); } else if ("json".equals(format)) { if (!HEAD.equals(httpServletRequest.getMethod())) { From b63490c08492bd595300e10e9b3a671fe242a23b Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 7 May 2019 15:10:54 -0600 Subject: [PATCH 2/6] Implemented allowed methods and content negotiation for X-TC-Format --- .../core/http/RouterFilter.java | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/RouterFilter.java b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/RouterFilter.java index 80200061de..7972373ba4 100644 --- a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/RouterFilter.java +++ b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/RouterFilter.java @@ -44,6 +44,7 @@ public class RouterFilter extends OncePerRequestFilter { public static final String REDIRECT_QUERY_PARAM = "trred"; public static final String FORMAT_HEADER = "X-TC-Format"; private static final String HEAD = "HEAD"; + public static final String GET = "GET"; @Autowired private TrafficRouterManager trafficRouterManager; @@ -187,13 +188,29 @@ private void setSingleResponse(final HTTPRouteResult routeResult, final HttpServ } httpAccessRecordBuilder.responseCode(HttpServletResponse.SC_OK); - } else if ("json".equals(formatHdr) || "application/json".equals(formatHdr)) { - response.setContentType("application/json"); - if (!HEAD.equals(httpServletRequest.getMethod())) { - response.getWriter().println(routeResult.toMultiLocationJSONString()); - httpAccessRecordBuilder.responseURLs(routeResult.getUrls()); + + } else if (formatHdr != null) { + if ("json".equals(formatHdr) || "application/json".equals(formatHdr)) { + if (HEAD.equals(httpServletRequest.getMethod()) || GET.equals(httpServletRequest.getMethod())) { + response.setContentType("application/json"); + final String resp = routeResult.toMultiLocationJSONString(); + if (!HEAD.equals(httpServletRequest.getMethod())) { + response.getWriter().println(resp); + httpAccessRecordBuilder.responseURLs(routeResult.getUrls()); + } else { + response.setHeader("Content-Length", Integer.toString(resp.length())); + } + httpAccessRecordBuilder.responseCode(HttpServletResponse.SC_OK); + } else { + response.setHeader("Allow", GET + ',' + HEAD); + httpAccessRecordBuilder.responseCode(HttpServletResponse.SC_METHOD_NOT_ALLOWED); + } + } else { + response.setContentType("text/plain"); + response.getWriter().println("Unsupported target format: " + formatHdr); + httpAccessRecordBuilder.responseCode(HttpServletResponse.SC_NOT_ACCEPTABLE); } - httpAccessRecordBuilder.responseCode(HttpServletResponse.SC_OK); + } else if ("json".equals(format)) { if (!HEAD.equals(httpServletRequest.getMethod())) { response.setContentType("application/json"); From a4014511e5731c14b2ec07c9dd861b10cb867007 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Wed, 8 May 2019 08:23:28 -0600 Subject: [PATCH 3/6] Added docs for format/X-TC-Format --- docs/source/admin/traffic_router.rst | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/source/admin/traffic_router.rst b/docs/source/admin/traffic_router.rst index c9ca86e629..81cd597daa 100644 --- a/docs/source/admin/traffic_router.rst +++ b/docs/source/admin/traffic_router.rst @@ -712,6 +712,32 @@ The following needs to be completed for Steering to work correctly: .. seealso:: For more information see :ref:`steering-qht`. +Client-Controlled Steering +-------------------------- +While normally this would refer to "CLIENT_STEERING" :term:`Delivery Service`\ s, it can also refer to methods provided to clients that can influence "Steering" behavior. There are a few ways this can be accomplished + +.. _trred: + +``trred`` +""""""""" +When a client requests resolution from a Steering :term:`Delivery Service`, they may optionally provide the ``trred`` Query Parameter - e.g. ``http://video.demo1.mycdn.ciab.test/?trred=false``. If provided, its value must be exactly ``false`` (case-insensitive) to have any effect. When this happens, Traffic Router will respond with a ``200 OK`` HTTP response (as opposed to the standard ``302 Moved Temporarily`` with associated ``Location``), and the response body can be expected to contain a list of "targets" from which the client may choose. + +``X-TC-Steering-Option`` +"""""""""""""""""""""""" +Clients may provide an ``X-TC-Steering-Option`` header with a value set to the ``xml_id`` of the desired "target" - thus bypassing Steering behavior. + +``format``/``X-TC-Format`` +"""""""""""""""""""""""""" +In cases where multiple "targets" are presented to the client, the formatting of the target list can be adjusted by either a Query Parameter or an HTTP header. In the case of the ``format`` query parameter, the only permissible value is ``json`` (case-sensitive). Failure to provide exactly that value will result in the parameter being ignored entirely - the response will be the same as if the parameter were not provided at all. e.g. ``http://video.demo1.mycdn.ciab.test/?format=json`` will result in a JSON-encoded response, while ``http://video.demo1.mycdn.ciab.test?format=yaml`` is ignored entirely. + +In the case of the ``X-TC-Format`` header, exactly two values are permissible: ``json`` and its MIME-Type equivalent :mimetype:`application/json`. + +.. note:: ``format`` and ``X-TC-Format`` only make sense on STEERING :term:`Delivery Service`\ s (since a CLIENT_STEERING :term:`Delivery Service` already provides the same information in the same encoding). Furthermore, they will cause Traffic Router to return a ``200 OK`` response (similar to :ref:`trred`'s behavior) containing *a single target* that would otherwise appear in a ``Location`` header. The only difference between ``format``/``X-TC-Format`` and :ref:`trred` is that the former will return only a single target while the latter returns all available targets as an array. + +.. warning:: While not strictly deprecated, it is recommended that, where possible, developers disregard the Query Parameter in favor of the HTTP header. This is because ``format`` is a fairly typical query parameter that may be used by origins (and JSON is an immensely popular encoding format), so using it may cause unintended side-effects. + +.. tip:: When using the :ref:`trred` Query Parameter, it is not necessary to use either ``format`` or ``X-TC-Format``; the response will be JSON-encoded by default. + HTTPS for HTTP Delivery Services ================================ .. versionadded:: 1.7 From 17a251f2fca7c47fcae6656899e82df042ad36de Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 14 May 2019 10:21:21 -0600 Subject: [PATCH 4/6] Added tests --- .../core/external/RouterTest.java | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/external/RouterTest.java b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/external/RouterTest.java index 232c4c7f32..4d465b2d60 100644 --- a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/external/RouterTest.java +++ b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/external/RouterTest.java @@ -625,6 +625,60 @@ public void itDoesUseLocationFormatResponse() throws IOException, InterruptedExc } } + @Test + public void itDoesUseLocationFormatHeader() throws IOException, InterruptedException { + + // queryparam-compatible format + HttpGet httpGet = new HttpGet("http://localhost:" + routerHttpPort + "/stuff?fakeClientIpAddress=12.34.56.78"); + httpGet.addHeader("Host", "tr." + deliveryServiceId + ".bar"); + httpGet.addHeader("X-TC-Format", "json"); + CloseableHttpResponse response = null; + + try { + response = httpClient.execute(httpGet); + assert response.getStatusLine().getStatusCode() == 200; + + HttpEntity entity = response.getEntity(); + + assert entity.getContent() != null; + System.out.println(entity.getContent()); + + JsonNode json = (new ObjectMapper(new JsonFactory())).readTree(entity.getContent()); + System.out.println(json); + + assert json.has("location"); + assert validLocations.contains(json.get("location").asText()); + assert json.get("location").asText().startsWith("http://")); + } finally { + if (response != null) response.close(); + } + + // MIME-type format + httpGet = new HttpGet("http://localhost:" + routerHttpPort + "/stuff?fakeClientIpAddress=12.34.56.78"); + httpGet.addHeader("Host", "tr." + deliveryServiceId + ".bar"); + httpGet.addHeader("X-TC-Format", "application/json"); + CloseableHttpResponse response = null; + + try { + response = httpClient.execute(httpGet); + assert response.getStatusLine().getStatusCode() == 200; + + HttpEntity entity = response.getEntity(); + + assert entity.getContent() != null; + System.out.println(entity.getContent()); + + JsonNode json = (new ObjectMapper(new JsonFactory())).readTree(entity.getContent()); + System.out.println(json); + + assert json.has("location"); + assert validLocations.contains(json.get("location").asText()); + assert json.get("location").asText().startsWith("http://")); + } finally { + if (response != null) response.close(); + } + } + @Test public void itDoesNotUseLocationFormatResponseForHead() throws IOException, InterruptedException { HttpHead httpHead = new HttpHead("http://localhost:" + routerHttpPort + "/stuff?fakeClientIpAddress=12.34.56.78&format=json"); From a36e3cea10314f6213aa3ef5822a3c3a49af1a14 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 16 May 2019 08:52:55 -0600 Subject: [PATCH 5/6] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e006dce29..768d205af8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -174,6 +174,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Added support for DS plugin parameters for cachekey, slice, cache_range_requests, background_fetch, url_sig as remap.config parameters. - Updated T3C changes in Ansible playbooks - Updated all endpoints in infrastructure code to use API version 2.0 +- Added HTTP Header equivalent of ``format`` query parameter in Traffic Router - ``X-TC-Format``. ### Fixed - [#5690](https://github.com/apache/trafficcontrol/issues/5690) - Fixed github action for added/modified db migration file. From 243bacccb18896ad4ffeff2609360755f060c644 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 30 Oct 2020 09:50:17 -0600 Subject: [PATCH 6/6] Fix TR tests compilation errors --- .../traffic_router/core/external/RouterTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/external/RouterTest.java b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/external/RouterTest.java index 4d465b2d60..39afaf4d3d 100644 --- a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/external/RouterTest.java +++ b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/external/RouterTest.java @@ -648,7 +648,7 @@ public void itDoesUseLocationFormatHeader() throws IOException, InterruptedExcep assert json.has("location"); assert validLocations.contains(json.get("location").asText()); - assert json.get("location").asText().startsWith("http://")); + assert json.get("location").asText().startsWith("http://"); } finally { if (response != null) response.close(); } @@ -657,7 +657,7 @@ public void itDoesUseLocationFormatHeader() throws IOException, InterruptedExcep httpGet = new HttpGet("http://localhost:" + routerHttpPort + "/stuff?fakeClientIpAddress=12.34.56.78"); httpGet.addHeader("Host", "tr." + deliveryServiceId + ".bar"); httpGet.addHeader("X-TC-Format", "application/json"); - CloseableHttpResponse response = null; + response = null; try { response = httpClient.execute(httpGet); @@ -673,7 +673,7 @@ public void itDoesUseLocationFormatHeader() throws IOException, InterruptedExcep assert json.has("location"); assert validLocations.contains(json.get("location").asText()); - assert json.get("location").asText().startsWith("http://")); + assert json.get("location").asText().startsWith("http://"); } finally { if (response != null) response.close(); }