From 7dcb0fa8a36d72e6b1a21f5d84091e8d157e5fc3 Mon Sep 17 00:00:00 2001 From: Jasmine Emanouel Date: Fri, 18 Oct 2024 13:30:31 +1100 Subject: [PATCH 1/3] Fix for #11784 --- src/proxy/http/HttpTransact.cc | 10 +- .../headers/cachedDuplicateHeaders.test.py | 60 ++++ .../headers/replays/cache-test.replay.yaml | 257 ++++++++++++++++++ 3 files changed, 323 insertions(+), 4 deletions(-) create mode 100644 tests/gold_tests/headers/cachedDuplicateHeaders.test.py create mode 100644 tests/gold_tests/headers/replays/cache-test.replay.yaml diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc index c6fb868cb9c..cb7d76abb26 100644 --- a/src/proxy/http/HttpTransact.cc +++ b/src/proxy/http/HttpTransact.cc @@ -5023,8 +5023,8 @@ HttpTransact::merge_response_header_with_cached_header(HTTPHdr *cached_header, H // may be a dup we've already copied in. If // dups show up we look through the remaining // header fields in the new response, nuke - // them in the cached response and then add in - // the remaining fields one by one from the + // the dup headers in the cached response and then + // add them back in one by one from the // response header // if (field.m_next_dup) { @@ -5038,8 +5038,10 @@ HttpTransact::merge_response_header_with_cached_header(HTTPHdr *cached_header, H for (auto spot2 = spot; spot2 != limit; ++spot2) { MIMEField &field2{*spot2}; name2 = field2.name_get(&name_len2); - - cached_header->field_delete(name2, name_len2); + // Only delete the duplicated headers + if (name2 == name) { + cached_header->field_delete(name2, name_len2); + } } dups_seen = true; } diff --git a/tests/gold_tests/headers/cachedDuplicateHeaders.test.py b/tests/gold_tests/headers/cachedDuplicateHeaders.test.py new file mode 100644 index 00000000000..c3c6be98d7f --- /dev/null +++ b/tests/gold_tests/headers/cachedDuplicateHeaders.test.py @@ -0,0 +1,60 @@ +''' +Test cached responses and requests with bodies +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF 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. + +Test.Summary = ''' +Test revalidating cached objects +''' +testName = "RevalidateCacheObject" +Test.ContinueOnFail = True + + +class CachedHeaderValidationTest: + replay_file = "replays/cache-test.replay.yaml" + + def __init__(self): + self.setupOriginServer() + self.setupTS() + + def setupOriginServer(self): + self.server = Test.MakeVerifierServerProcess("cached-header-verifier-server", self.replay_file) + + def setupTS(self): + self.ts = Test.MakeATSProcess("ts", enable_tls=True) + self.ts.Disk.plugin_config.AddLine('xdebug.so --enable=x-cache,x-cache-key,via') + self.ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http', + 'proxy.config.http.response_via_str': 3, + }) + self.ts.Disk.remap_config.AddLine('map / http://127.0.0.1:{0}'.format(self.server.Variables.http_port)) + + def runTraffic(self): + tr = Test.AddTestRun() + tr.AddVerifierClientProcess("cached-header-verifier-client", self.replay_file, http_ports=[self.ts.Variables.port]) + tr.Processes.Default.StartBefore(self.server) + tr.Processes.Default.StartBefore(self.ts) + tr.StillRunningAfter = self.ts + tr.StillRunningAfter = self.server + + def run(self): + self.runTraffic() + + +CachedHeaderValidationTest().run() diff --git a/tests/gold_tests/headers/replays/cache-test.replay.yaml b/tests/gold_tests/headers/replays/cache-test.replay.yaml new file mode 100644 index 00000000000..df882262002 --- /dev/null +++ b/tests/gold_tests/headers/replays/cache-test.replay.yaml @@ -0,0 +1,257 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF 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. + +meta: + version: "1.0" + +# Note 1: +# When testing duplicate headers here, replay files cannot +# handle two seperate values. So make sure that duplicate headers +# have the same value in testing. + +sessions: + - transactions: + # Test 0: Cache fill with content-type after duplicate headers + - client-request: + method: "GET" + version: "1.1" + url: /1 + headers: + fields: + - [Host, example.com] + - [uuid, fill_1] + - [x-debug, "x-cache,x-cache-key,via"] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Date, "Mon, 23 Sep 2024 14:22:14 GMT"] + - [X-Reveal-Duplicate, same] + - [Cache-Control, "max-age=1, stale-if-error=1, stale-while-revalidate=1, public"] + - [Content-Length, 3] + - [Content-Type, application/javascript] + - [Expires, "Mon, 23 Sep 2024 14:22:44 GMT"] + - [Last-Modified, "Mon, 23 Sep 2024 14:22:14 GMT"] + - [X-Reveal-Duplicate, same] + + proxy-response: + status: 200 + reason: OK + headers: + fields: + - [Date, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Reveal-Duplicate, { value: same, as: equal }] + - [Cache-Control, { value: "max-age=1, stale-if-error=1, stale-while-revalidate=1, public", as: equal }] + - [Content-Length, { value: 3, as: equal }] + - [Content-Type, { value: application/javascript, as: equal }] + - [Expires, { value: "Mon, 23 Sep 2024 14:22:44 GMT", as: equal }] + - [Last-Modified, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Cache, { value: miss, as: equal }] + - [X-Reveal-Duplicate, { value: same, as: equal }] + + # Test 1: Cache hit stale with content-type after duplicate headers + - client-request: + method: "GET" + version: "1.1" + url: /1 + headers: + fields: + - [Host, example.com] + - [uuid, stale_1] + - [x-debug, "x-cache,x-cache-key,via"] + + server-response: + status: 304 + reason: Not Modified + headers: + fields: + - [Date, "Mon, 23 Sep 2024 14:22:14 GMT"] + - [X-Reveal-Duplicate, same] + - [Cache-Control, "max-age=1, stale-if-error=1, stale-while-revalidate=1, public"] + - [Content-Length, 3] + - [Content-Type, application/javascript] + - [Expires, "Mon, 23 Sep 2024 14:22:44 GMT"] + - [Last-Modified, "Mon, 23 Sep 2024 14:22:14 GMT"] + - [X-Reveal-Duplicate, same] + + proxy-response: + status: 200 + reason: OK + headers: + fields: + - [Date, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Reveal-Duplicate, { value: same, as: equal }] + - [Cache-Control, { value: "max-age=1, stale-if-error=1, stale-while-revalidate=1, public", as: equal }] + - [Content-Length, { value: 3, as: equal }] + - [Content-Type, { value: application/javascript, as: equal }] + - [Expires, { value: "Mon, 23 Sep 2024 14:22:44 GMT", as: equal }] + - [Last-Modified, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Cache, { value: hit-stale, as: equal }] + - [X-Reveal-Duplicate, { value: same, as: equal }] + + # Test 2: Cache fill with content-type before duplicate headers + - client-request: + method: "GET" + version: "1.1" + url: /2 + headers: + fields: + - [Host, example.com] + - [uuid, fill_2] + - [x-debug, "x-cache,x-cache-key,via"] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Date, "Mon, 23 Sep 2024 14:22:14 GMT"] + - [Content-Type, application/javascript] + - [X-Reveal-Duplicate, same] + - [Cache-Control, "max-age=1, stale-if-error=1, stale-while-revalidate=1, public"] + - [Content-Length, 3] + - [Expires, "Mon, 23 Sep 2024 14:22:44 GMT"] + - [Last-Modified, "Mon, 23 Sep 2024 14:22:14 GMT"] + - [X-Reveal-Duplicate, same] + + proxy-response: + status: 200 + reason: OK + headers: + fields: + - [Date, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Reveal-Duplicate, { value: same, as: equal }] + - [Cache-Control, { value: "max-age=1, stale-if-error=1, stale-while-revalidate=1, public", as: equal }] + - [Content-Length, { value: 3, as: equal }] + - [Content-Type, { value: application/javascript, as: equal }] + - [Expires, { value: "Mon, 23 Sep 2024 14:22:44 GMT", as: equal }] + - [Last-Modified, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Cache, { value: miss, as: equal }] + - [X-Reveal-Duplicate, { value: same, as: equal }] + + # Test 3: Cache hit stale with content-type before duplicate headers + - client-request: + method: "GET" + version: "1.1" + url: /2 + headers: + fields: + - [Host, example.com] + - [uuid, stale_2] + - [x-debug, "x-cache,x-cache-key,via"] + + server-response: + status: 304 + reason: Not Modified + headers: + fields: + - [Date, "Mon, 23 Sep 2024 14:22:14 GMT"] + - [Content-Type, application/javascript] + - [X-Reveal-Duplicate, same] + - [X-Reveal-Duplicate, same] + - [Cache-Control, "max-age=1, stale-if-error=1, stale-while-revalidate=1, public"] + - [Content-Length, 3] + - [Expires, "Mon, 23 Sep 2024 14:22:44 GMT"] + - [Last-Modified, "Mon, 23 Sep 2024 14:22:14 GMT"] + + proxy-response: + status: 200 + reason: OK + headers: + fields: + - [Date, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Reveal-Duplicate, { value: same, as: equal }] + - [X-Reveal-Duplicate, { value: same, as: equal }] + - [Cache-Control, { value: "max-age=1, stale-if-error=1, stale-while-revalidate=1, public", as: equal }] + - [Content-Length, { value: 3, as: equal }] + - [Content-Type, { value: application/javascript, as: equal }] + - [Expires, { value: "Mon, 23 Sep 2024 14:22:44 GMT", as: equal }] + - [Last-Modified, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Cache, { value: hit-stale, as: equal }] + + # Test 4: Cache fill with no duplicate headers + - client-request: + method: "GET" + version: "1.1" + url: /3 + headers: + fields: + - [Host, example.com] + - [uuid, fill_3] + - [x-debug, "x-cache,x-cache-key,via"] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Date, "Mon, 23 Sep 2024 14:22:14 GMT"] + - [Content-Type, application/javascript] + - [Cache-Control, "max-age=1, stale-if-error=1, stale-while-revalidate=1, public"] + - [Content-Length, 3] + - [Expires, "Mon, 23 Sep 2024 14:22:44 GMT"] + - [Last-Modified, "Mon, 23 Sep 2024 14:22:14 GMT"] + + proxy-response: + status: 200 + reason: OK + headers: + fields: + - [Date, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [Cache-Control, { value: "max-age=1, stale-if-error=1, stale-while-revalidate=1, public", as: equal }] + - [Content-Length, { value: 3, as: equal }] + - [Content-Type, { value: application/javascript, as: equal }] + - [Expires, { value: "Mon, 23 Sep 2024 14:22:44 GMT", as: equal }] + - [Last-Modified, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Cache, { value: miss, as: equal }] + + # Test 5: Cache hit stale no duplicate headers + - client-request: + method: "GET" + version: "1.1" + url: /3 + headers: + fields: + - [Host, example.com] + - [uuid, stale_3] + - [x-debug, "x-cache,x-cache-key,via"] + + server-response: + status: 304 + reason: Not Modified + headers: + fields: + - [Date, "Mon, 23 Sep 2024 14:22:14 GMT"] + - [Content-Type, application/javascript] + - [Cache-Control, "max-age=1, stale-if-error=1, stale-while-revalidate=1, public"] + - [Content-Length, 3] + - [Expires, "Mon, 23 Sep 2024 14:22:44 GMT"] + - [Last-Modified, "Mon, 23 Sep 2024 14:22:14 GMT"] + + proxy-response: + status: 200 + reason: OK + headers: + fields: + - [Date, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [Cache-Control, { value: "max-age=1, stale-if-error=1, stale-while-revalidate=1, public", as: equal }] + - [Content-Length, { value: 3, as: equal }] + - [Content-Type, { value: application/javascript, as: equal }] + - [Expires, { value: "Mon, 23 Sep 2024 14:22:44 GMT", as: equal }] + - [Last-Modified, { value: "Mon, 23 Sep 2024 14:22:14 GMT", as: equal }] + - [X-Cache, { value: hit-stale, as: equal }] From 7ef85f03136c10e683045d3ac1c87c1b17931d3a Mon Sep 17 00:00:00 2001 From: Jasmine Emanouel Date: Mon, 21 Oct 2024 10:30:51 +1100 Subject: [PATCH 2/3] Just specify content-type not be altered --- src/proxy/http/HttpTransact.cc | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc index cb7d76abb26..310808960fb 100644 --- a/src/proxy/http/HttpTransact.cc +++ b/src/proxy/http/HttpTransact.cc @@ -5038,10 +5038,19 @@ HttpTransact::merge_response_header_with_cached_header(HTTPHdr *cached_header, H for (auto spot2 = spot; spot2 != limit; ++spot2) { MIMEField &field2{*spot2}; name2 = field2.name_get(&name_len2); - // Only delete the duplicated headers - if (name2 == name) { - cached_header->field_delete(name2, name_len2); + + // It is specified above that content type should not + // be altered here however when a duplicate header + // is present, all headers following are delete and + // re-added back. This includes content type if it follows + // any duplicate header. This leads to the loss of + // content type in the client response. + // This ensures that it is not altered when duplicate + // headers are present. + if (name2 == MIME_FIELD_CONTENT_TYPE) { + continue; } + cached_header->field_delete(name2, name_len2); } dups_seen = true; } From 03285837468c1c1a39520f8c55c1ce1116b227e2 Mon Sep 17 00:00:00 2001 From: Jasmine Emanouel <40879549+jasmine-nahrain@users.noreply.github.com> Date: Mon, 21 Oct 2024 15:45:50 +1100 Subject: [PATCH 3/3] Update HttpTransact.cc --- src/proxy/http/HttpTransact.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc index 310808960fb..98f8457e333 100644 --- a/src/proxy/http/HttpTransact.cc +++ b/src/proxy/http/HttpTransact.cc @@ -5023,8 +5023,8 @@ HttpTransact::merge_response_header_with_cached_header(HTTPHdr *cached_header, H // may be a dup we've already copied in. If // dups show up we look through the remaining // header fields in the new response, nuke - // the dup headers in the cached response and then - // add them back in one by one from the + // them in the cached response and then add in + // the remaining fields one by one from the // response header // if (field.m_next_dup) {