diff --git a/plugins/compress/compress.cc b/plugins/compress/compress.cc index 44e8235a0d0..7cc9a5a3d86 100644 --- a/plugins/compress/compress.cc +++ b/plugins/compress/compress.cc @@ -354,7 +354,7 @@ compress_transform_init(TSCont contp, Data *data) } if (content_encoding_header(bufp, hdr_loc, data->compression_type, data->compression_algorithms) == TS_SUCCESS && - vary_header(bufp, hdr_loc) == TS_SUCCESS && etag_header(bufp, hdr_loc) == TS_SUCCESS) { + etag_header(bufp, hdr_loc) == TS_SUCCESS) { downstream_conn = TSTransformOutputVConnGet(contp); data->downstream_buffer = TSIOBufferCreate(); data->downstream_reader = TSIOBufferReaderAlloc(data->downstream_buffer); @@ -685,7 +685,7 @@ compress_transform(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */) } static int -transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration, int *compress_type, int *algorithms) +is_content_compressible(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration) { /* Server response header */ TSMBuffer bufp; @@ -695,7 +695,6 @@ transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration /* Client request header */ TSMBuffer cbuf; TSMLoc chdr; - TSMLoc cfield; const char *value; int len; @@ -737,6 +736,100 @@ transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration return 0; } + // the only compressible method is currently GET or POST. + int method_length; + const char *method = TSHttpHdrMethodGet(cbuf, chdr, &method_length); + + if (!((method_length == TS_HTTP_LEN_GET && memcmp(method, TS_HTTP_METHOD_GET, TS_HTTP_LEN_GET) == 0) || + (method_length == TS_HTTP_LEN_POST && memcmp(method, TS_HTTP_METHOD_POST, TS_HTTP_LEN_POST) == 0))) { + debug("method is not GET or POST, not compressible"); + TSHandleMLocRelease(cbuf, TS_NULL_MLOC, chdr); + TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); + return 0; + } + + TSHandleMLocRelease(cbuf, TS_NULL_MLOC, chdr); + + /* If there already exists a content encoding then we don't want + to do anything. */ + field_loc = TSMimeHdrFieldFind(bufp, hdr_loc, TS_MIME_FIELD_CONTENT_ENCODING, -1); + if (field_loc) { + info("response is already content encoded, not compressible"); + TSHandleMLocRelease(bufp, hdr_loc, field_loc); + TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); + return 0; + } + + field_loc = TSMimeHdrFieldFind(bufp, hdr_loc, TS_MIME_FIELD_CONTENT_LENGTH, TS_MIME_LEN_CONTENT_LENGTH); + if (field_loc != TS_NULL_MLOC) { + unsigned int hdr_value = TSMimeHdrFieldValueUintGet(bufp, hdr_loc, field_loc, -1); + TSHandleMLocRelease(bufp, hdr_loc, field_loc); + if (hdr_value == 0) { + info("response is 0-length, not compressible"); + TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); + return 0; + } + + if (hdr_value < host_configuration->minimum_content_length()) { + info("response is smaller than minimum content length, not compressing"); + TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); + return 0; + } + } + + // Check if content type is compressible based on configuration. + field_loc = TSMimeHdrFieldFind(bufp, hdr_loc, TS_MIME_FIELD_CONTENT_TYPE, -1); + if (!field_loc) { + info("no content type header found, not compressible"); + TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); + return 0; + } + + value = TSMimeHdrFieldValueStringGet(bufp, hdr_loc, field_loc, -1, &len); + + int rv = host_configuration->is_content_type_compressible(value, len); + + if (!rv) { + info("content-type [%.*s] not compressible", len, value); + } + + TSHandleMLocRelease(bufp, hdr_loc, field_loc); + TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); + + return rv; +} + +static int +client_accepts_compression(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration, int *compress_type, int *algorithms) +{ + /* Server response header */ + TSMBuffer bufp; + TSMLoc hdr_loc; + + /* Client request header */ + TSMBuffer cbuf; + TSMLoc chdr; + TSMLoc cfield; + + const char *value; + int len; + + if (server) { + if (TS_SUCCESS != TSHttpTxnServerRespGet(txnp, &bufp, &hdr_loc)) { + return 0; + } + } else { + if (TS_SUCCESS != TSHttpTxnCachedRespGet(txnp, &bufp, &hdr_loc)) { + return 0; + } + } + + if (TS_SUCCESS != TSHttpTxnClientReqGet(txnp, &cbuf, &chdr)) { + info("cound not get client request"); + TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); + return 0; + } + // check Partial Object is transformable if (host_configuration->range_request_ctl() == RangeRequestCtrl::NO_COMPRESSION) { // check Range header in client request @@ -759,21 +852,6 @@ transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration TSHandleMLocRelease(cbuf, TS_NULL_MLOC, chdr); return 0; } - - TSHandleMLocRelease(bufp, hdr_loc, content_range_hdr_field); - TSHandleMLocRelease(cbuf, chdr, range_hdr_field); - } - - // the only compressible method is currently GET. - int method_length; - const char *method = TSHttpHdrMethodGet(cbuf, chdr, &method_length); - - if (!((method_length == TS_HTTP_LEN_GET && memcmp(method, TS_HTTP_METHOD_GET, TS_HTTP_LEN_GET) == 0) || - (method_length == TS_HTTP_LEN_POST && memcmp(method, TS_HTTP_METHOD_POST, TS_HTTP_LEN_POST) == 0))) { - debug("method is not GET or POST, not compressible"); - TSHandleMLocRelease(cbuf, TS_NULL_MLOC, chdr); - TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); - return 0; } *algorithms = host_configuration->compression_algorithms(); @@ -807,10 +885,10 @@ transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration TSHandleMLocRelease(cbuf, chdr, cfield); TSHandleMLocRelease(cbuf, TS_NULL_MLOC, chdr); + TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); if (!compression_acceptable) { info("no acceptable encoding match found in request header, not compressible"); - TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); return 0; } } else { @@ -821,52 +899,48 @@ transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration return 0; } - /* If there already exists a content encoding then we don't want - to do anything. */ - field_loc = TSMimeHdrFieldFind(bufp, hdr_loc, TS_MIME_FIELD_CONTENT_ENCODING, -1); - if (field_loc) { - info("response is already content encoded, not compressible"); - TSHandleMLocRelease(bufp, hdr_loc, field_loc); - TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); - return 0; - } - - field_loc = TSMimeHdrFieldFind(bufp, hdr_loc, TS_MIME_FIELD_CONTENT_LENGTH, TS_MIME_LEN_CONTENT_LENGTH); - if (field_loc != TS_NULL_MLOC) { - unsigned int hdr_value = TSMimeHdrFieldValueUintGet(bufp, hdr_loc, field_loc, -1); - TSHandleMLocRelease(bufp, hdr_loc, field_loc); - if (hdr_value == 0) { - info("response is 0-length, not compressible"); - return 0; - } - - if (hdr_value < host_configuration->minimum_content_length()) { - info("response is smaller than minimum content length, not compressing"); - return 0; - } - } + return 1; +} - /* We only want to do gzip compression on documents that have a - content type of "text/" or "application/x-javascript". */ - field_loc = TSMimeHdrFieldFind(bufp, hdr_loc, TS_MIME_FIELD_CONTENT_TYPE, -1); - if (!field_loc) { - info("no content type header found, not compressible"); - TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); +static int +transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration, int *compress_type, int *algorithms, + bool *content_is_compressible) +{ + // First check if content could be compressible + *content_is_compressible = is_content_compressible(txnp, server, host_configuration); + if (!*content_is_compressible) { return 0; } - value = TSMimeHdrFieldValueStringGet(bufp, hdr_loc, field_loc, -1, &len); + // Then check if client accepts compression + return client_accepts_compression(txnp, server, host_configuration, compress_type, algorithms); +} - int rv = host_configuration->is_content_type_compressible(value, len); +static void +add_vary_header_for_compressible_content(TSHttpTxn txnp, bool server, HostConfiguration * /* hc ATS_UNUSED */) +{ + TSMBuffer resp_buf; + TSMLoc resp_loc; - if (!rv) { - info("content-type [%.*s] not compressible", len, value); + // Get the response headers + if (server) { + if (TS_SUCCESS != TSHttpTxnServerRespGet(txnp, &resp_buf, &resp_loc)) { + return; + } + } else { + if (TS_SUCCESS != TSHttpTxnCachedRespGet(txnp, &resp_buf, &resp_loc)) { + return; + } } - TSHandleMLocRelease(bufp, hdr_loc, field_loc); - TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); + // Add Vary: Accept-Encoding header + if (vary_header(resp_buf, resp_loc) != TS_SUCCESS) { + error("failed to add Vary header for compressible content"); + TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); + return; + } - return rv; + TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); } static void @@ -895,6 +969,21 @@ compress_transform_add(TSHttpTxn txnp, HostConfiguration *hc, int compress_type, TSHttpTxnHookAdd(txnp, TS_HTTP_RESPONSE_TRANSFORM_HOOK, connp); } +static void +handle_compression_and_vary(TSHttpTxn txnp, bool server, HostConfiguration *hc, int *compress_type, int *algorithms) +{ + // Check if content is compressible and add compression if client accepts it + bool content_is_compressible; + if (transformable(txnp, server, hc, compress_type, algorithms, &content_is_compressible)) { + compress_transform_add(txnp, hc, *compress_type, *algorithms); + } + + // Add Vary: Accept-Encoding for all compressible content to ensure proper HTTP caching + if (content_is_compressible) { + add_vary_header_for_compressible_content(txnp, server, hc); + } +} + HostConfiguration * find_host_configuration(TSHttpTxn /* txnp ATS_UNUSED */, TSMBuffer bufp, TSMLoc locp, Configuration *config) { @@ -939,9 +1028,7 @@ transform_plugin(TSCont contp, TSEvent event, void *edata) } } - if (transformable(txnp, true, hc, &compress_type, &algorithms)) { - compress_transform_add(txnp, hc, compress_type, algorithms); - } + handle_compression_and_vary(txnp, true, hc, &compress_type, &algorithms); } break; @@ -967,9 +1054,7 @@ transform_plugin(TSCont contp, TSEvent event, void *edata) if (TS_ERROR != TSHttpTxnCacheLookupStatusGet(txnp, &obj_status) && (TS_CACHE_LOOKUP_HIT_FRESH == obj_status)) { if (hc != nullptr) { info("handling compression of cached object"); - if (transformable(txnp, false, hc, &compress_type, &algorithms)) { - compress_transform_add(txnp, hc, compress_type, algorithms); - } + handle_compression_and_vary(txnp, false, hc, &compress_type, &algorithms); } } else { // Prepare for going to origin diff --git a/tests/gold_tests/pluginTest/compress/compress.gold b/tests/gold_tests/pluginTest/compress/compress.gold index 6745b65f0ae..7677295d987 100644 --- a/tests/gold_tests/pluginTest/compress/compress.gold +++ b/tests/gold_tests/pluginTest/compress/compress.gold @@ -3,8 +3,8 @@ > Accept-Encoding: gzip, deflate, sdch, br < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: br < Vary: Accept-Encoding +< Content-Encoding: br < Content-Length: 46 === > GET ``/obj0 HTTP/1.1 @@ -12,8 +12,8 @@ > Accept-Encoding: gzip < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > GET ``/obj0 HTTP/1.1 @@ -21,8 +21,8 @@ > Accept-Encoding: br < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: br < Vary: Accept-Encoding +< Content-Encoding: br < Content-Length: 46 === > GET ``/obj0 HTTP/1.1 @@ -31,14 +31,15 @@ < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 +< Vary: Accept-Encoding === > GET ``/obj1 HTTP/1.1 > X-Ats-Compress-Test: 1/gzip, deflate, sdch, br > Accept-Encoding: gzip, deflate, sdch, br < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > GET ``/obj1 HTTP/1.1 @@ -46,8 +47,8 @@ > Accept-Encoding: gzip < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > GET ``/obj1 HTTP/1.1 @@ -56,6 +57,7 @@ < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 +< Vary: Accept-Encoding === > GET ``/obj1 HTTP/1.1 > X-Ats-Compress-Test: 1/deflate @@ -63,14 +65,15 @@ < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 +< Vary: Accept-Encoding === > GET ``/obj2 HTTP/1.1 > X-Ats-Compress-Test: 2/gzip, deflate, sdch, br > Accept-Encoding: gzip, deflate, sdch, br < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: br < Vary: Accept-Encoding +< Content-Encoding: br < Content-Length: 46 === > GET ``/obj2 HTTP/1.1 @@ -78,8 +81,8 @@ > Accept-Encoding: gzip < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > GET ``/obj2 HTTP/1.1 @@ -87,8 +90,8 @@ > Accept-Encoding: br < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: br < Vary: Accept-Encoding +< Content-Encoding: br < Content-Length: 46 === > GET ``/obj2 HTTP/1.1 @@ -97,14 +100,15 @@ < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 +< Vary: Accept-Encoding === > GET ``/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip;q=0.666 > Accept-Encoding: gzip;q=0.666 < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > GET ``/obj0 HTTP/1.1 @@ -112,8 +116,8 @@ > Accept-Encoding: gzip;q=0.666x < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > GET ``/obj0 HTTP/1.1 @@ -121,8 +125,8 @@ > Accept-Encoding: gzip;q=#0.666 < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > GET ``/obj0 HTTP/1.1 @@ -130,24 +134,25 @@ > Accept-Encoding: gzip; Q = 0.666 < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === -> GET ``obj0 HTTP/1.1 +> GET ``/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip;q=0.0 > Accept-Encoding: gzip;q=0.0 < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 +< Vary: Accept-Encoding === -> GET ``obj0 HTTP/1.1 +> GET ``/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip;q=-0.1 > Accept-Encoding: gzip;q=-0.1 < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > GET ``/obj0 HTTP/1.1 @@ -155,8 +160,8 @@ > Accept-Encoding: aaa, gzip;q=0.666, bbb < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > GET ``/obj0 HTTP/1.1 @@ -164,8 +169,8 @@ > Accept-Encoding: br ; q=0.666, bbb < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: br < Vary: Accept-Encoding +< Content-Encoding: br < Content-Length: 46 === > GET ``/obj0 HTTP/1.1 @@ -173,8 +178,8 @@ > Accept-Encoding: aaa, gzip;q=0.666 , < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === > POST ``/obj3 HTTP/1.1 @@ -182,7 +187,7 @@ > Accept-Encoding: gzip < HTTP/1.1 200 OK < Content-Type: text/javascript -< Content-Encoding: gzip < Vary: Accept-Encoding +< Content-Encoding: gzip < Content-Length: 7`` === diff --git a/tests/gold_tests/pluginTest/compress/compress.test.py b/tests/gold_tests/pluginTest/compress/compress.test.py index fd34c6f7524..bdacacb69e8 100644 --- a/tests/gold_tests/pluginTest/compress/compress.test.py +++ b/tests/gold_tests/pluginTest/compress/compress.test.py @@ -253,6 +253,31 @@ def get_verify_command(out_path, decrompressor): tr.ReturnCode = 0 tr.Processes.Default.Command = get_verify_command(out_path, "gunzip -k") +# Test Vary header: compressible content without Accept-Encoding should get Vary: Accept-Encoding +tr = Test.AddTestRun('vary header test: no accept-encoding') +tr.Processes.Default.ReturnCode = 0 +out_path = get_out_path() +tr.MakeCurlCommand( + f"-o {out_path} --verbose --proxy http://127.0.0.1:{ts.Variables.port}" + f" --header 'X-Ats-Compress-Test: vary-no-accept-encoding'" + f" 'http://ae-0/obj0'" + " 2>> compress_vary.log", + ts=ts) + +# Test Vary header: compressible content with unsupported Accept-Encoding should get Vary: Accept-Encoding +tr = Test.AddTestRun('vary header test: unsupported accept-encoding') +tr.Processes.Default.ReturnCode = 0 +out_path = get_out_path() +tr.MakeCurlCommand(curl(ts, 0, "compress, identity", out_path).replace("compress_long.log", "compress_vary.log"), ts=ts) + +# Verify Vary header is present in both cases +tr = Test.AddTestRun('verify vary headers') +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Command = ( + r"tr -d '\r' < compress_vary.log | grep -i 'vary:.*accept-encoding' | sort > compress_vary_short.log") +f = tr.Disk.File("compress_vary_short.log") +f.Content = "compress_vary.gold" + # compress_long.log contains all the output from the curl commands. The tr removes the carriage returns for easier # readability. Curl seems to have a bug, where it will neglect to output an end of line before outputting an HTTP # message header line. The sed command is a work-around for this problem. greplog.sh uses the grep command to diff --git a/tests/gold_tests/pluginTest/compress/compress_userver.gold b/tests/gold_tests/pluginTest/compress/compress_userver.gold index 1ddc6a4f3b5..05d6c88416f 100644 --- a/tests/gold_tests/pluginTest/compress/compress_userver.gold +++ b/tests/gold_tests/pluginTest/compress/compress_userver.gold @@ -20,3 +20,5 @@ 0/ br ; q=0.666, bbb 0/aaa, gzip;q=0.666 , 3/gzip +vary-no-accept-encoding +0/compress, identity diff --git a/tests/gold_tests/pluginTest/compress/compress_vary.gold b/tests/gold_tests/pluginTest/compress/compress_vary.gold new file mode 100644 index 00000000000..1ba5ad4743f --- /dev/null +++ b/tests/gold_tests/pluginTest/compress/compress_vary.gold @@ -0,0 +1,2 @@ +< Vary: Accept-Encoding +< Vary: Accept-Encoding