This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new ae6819c8d Revert "Fixes silent header duplication that occurs upon
successful revalidation when duplicate headers are present (#9527)"
ae6819c8d is described below
commit ae6819c8d929099cdc2eaa3aaf48f5724ad6d18d
Author: Leif Hedstrom <[email protected]>
AuthorDate: Wed Apr 5 15:00:04 2023 -0600
Revert "Fixes silent header duplication that occurs upon successful
revalidation when duplicate headers are present (#9527)"
This reverts commit c7ed799ccc16c7cebbd8ce2b5951b9911bf65530.
Instead, we'll do #9534
---
proxy/http/HttpTransact.cc | 8 +-
proxy/http/Makefile.am | 29 +-
proxy/http/unit_tests/main.cc | 60 ---
proxy/http/unit_tests/test_HttpTransact.cc | 573 -----------------------------
4 files changed, 2 insertions(+), 668 deletions(-)
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 39e78c9c2..b2c1677a8 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -5109,17 +5109,11 @@
HttpTransact::merge_response_header_with_cached_header(HTTPHdr *cached_header, H
//
if (field.m_next_dup) {
if (dups_seen == false) {
- const char *name2;
- int name_len2;
-
// use a second iterator to delete the
// remaining response headers in the cached response,
// so that they will be added in the next iterations.
for (auto spot2 = spot; spot2 != limit; ++spot2) {
- MIMEField &field2{*spot2};
- name2 = field2.name_get(&name_len2);
-
- cached_header->field_delete(name2, name_len2);
+ cached_header->field_delete(&*spot2, true);
}
dups_seen = true;
}
diff --git a/proxy/http/Makefile.am b/proxy/http/Makefile.am
index 8afa4bd0d..53f983251 100644
--- a/proxy/http/Makefile.am
+++ b/proxy/http/Makefile.am
@@ -85,7 +85,7 @@ if BUILD_TESTS
libhttp_a_SOURCES += RegressionHttpTransact.cc
endif
-check_PROGRAMS = test_proxy_http test_PreWarm test_HttpTransact
+check_PROGRAMS = test_proxy_http test_PreWarm
TESTS = $(check_PROGRAMS)
@@ -123,33 +123,6 @@ test_PreWarm_LDADD = \
test_PreWarm_SOURCES = \
unit_tests/test_PreWarm.cc
-test_HttpTransact_CPPFLAGS = \
- $(AM_CPPFLAGS) \
- -I$(abs_top_srcdir)/tests/include
-
-if OS_LINUX
-test_HttpTransact_LDFLAGS = $(AM_LDFLAGS)\
- -Wl,--unresolved-symbols=ignore-all
-else
-test_HttpTransact_LDFLAGS = $(AM_LDFLAGS)\
- -Wl,-undefined -Wl,suppress -Wl,-flat_namespace -Wl,-dead_strip
-endif
-
-test_HttpTransact_LDADD = \
- $(top_builddir)/src/tscore/libtscore.la \
- $(top_builddir)/src/records/librecords_p.a \
- $(top_builddir)/proxy/ProxySession.o \
- $(top_builddir)/proxy/http/HttpTransact.o \
- $(top_builddir)/proxy/http/HttpTransactHeaders.o \
- $(top_builddir)/proxy/hdrs/libhdrs.a \
- $(top_builddir)/iocore/eventsystem/libinkevent.a \
- @SWOC_LIBS@
-
-test_HttpTransact_SOURCES = \
- ../../iocore/cache/test/stub.cc \
- unit_tests/main.cc \
- unit_tests/test_HttpTransact.cc
-
clang-tidy-local: $(libhttp_a_SOURCES) $(noinst_HEADERS)
$(CXX_Clang_Tidy)
diff --git a/proxy/http/unit_tests/main.cc b/proxy/http/unit_tests/main.cc
deleted file mode 100644
index 334de97c4..000000000
--- a/proxy/http/unit_tests/main.cc
+++ /dev/null
@@ -1,60 +0,0 @@
-/** @file
-
- The main file for tests
-
- @section license License
-
- 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.
-*/
-
-#define CATCH_CONFIG_MAIN
-#include "catch.hpp"
-
-#include "tscore/I_Layout.h"
-
-#include "I_EventSystem.h"
-#include "records/I_RecordsConfig.h"
-
-#include "diags.i"
-
-#define TEST_THREADS 1
-
-struct EventProcessorListener : Catch::TestEventListenerBase {
- using TestEventListenerBase::TestEventListenerBase;
-
- void
- testRunStarting(Catch::TestRunInfo const & /* testRunInfo */) override
- {
- Layout::create();
- init_diags("", nullptr);
- RecProcessInit();
- LibRecordsConfigInit();
-
- ink_event_system_init(EVENT_SYSTEM_MODULE_PUBLIC_VERSION);
- eventProcessor.start(TEST_THREADS);
-
- EThread *main_thread = new EThread;
- main_thread->set_specific();
- }
-
- void
- testRunEnded(Catch::TestRunStats const & /* testRunStats */) override
- {
- }
-};
-
-CATCH_REGISTER_LISTENER(EventProcessorListener);
diff --git a/proxy/http/unit_tests/test_HttpTransact.cc
b/proxy/http/unit_tests/test_HttpTransact.cc
deleted file mode 100644
index caa07e9e7..000000000
--- a/proxy/http/unit_tests/test_HttpTransact.cc
+++ /dev/null
@@ -1,573 +0,0 @@
-/** @file
-
- Unit Tests for HttpTransact
-
- @section license License
-
- 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.
- */
-
-#include <string_view>
-#include "tscore/Diags.h"
-#include "HttpTransact.h"
-#include "records/I_RecordsConfig.h"
-
-#include "catch.hpp"
-
-TEST_CASE("HttpTransact", "[http]")
-{
- url_init();
- mime_init();
- http_init();
-
- SECTION("HttpTransact::merge_response_header_with_cached_header")
- {
- SECTION("Basic")
- {
- HTTPHdr hdr1;
- HTTPHdr hdr2;
- MIMEField *field;
- const char *str;
- int len;
-
- struct header {
- std::string_view name;
- std::string_view value;
- };
-
- struct header input1[] = {
- {"AAA", "111"},
- {"BBB", "222"},
- {"CCC", "333"},
- };
- struct header input2[] = {
- {"DDD", "444"},
- {"EEE", "555"},
- {"FFF", "666"}
- };
-
- hdr1.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input1) {
- field = hdr1.field_create(entry.name.data(), entry.name.length());
- hdr1.field_attach(field);
- hdr1.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- hdr2.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input2) {
- field = hdr2.field_create(entry.name.data(), entry.name.length());
- hdr2.field_attach(field);
- hdr2.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- HttpTransact::merge_response_header_with_cached_header(&hdr1, &hdr2);
-
- CHECK(hdr1.fields_count() == 6);
-
- field = hdr1.field_find("AAA", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "111", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("BBB", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "222", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("CCC", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "333", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("DDD", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "444", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("EEE", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "555", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("FFF", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "666", len) == 0);
- CHECK(field->has_dups() == false);
- }
-
- SECTION("Have comon headers")
- {
- HTTPHdr hdr1;
- HTTPHdr hdr2;
- MIMEField *field;
- const char *str;
- int len;
-
- struct header {
- std::string_view name;
- std::string_view value;
- };
-
- struct header input1[] = {
- {"AAA", "111"},
- {"BBB", "222"},
- {"CCC", "333"},
- };
- struct header input2[] = {
- {"DDD", "444"},
- {"BBB", "555"},
- {"FFF", "666"}
- };
-
- hdr1.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input1) {
- field = hdr1.field_create(entry.name.data(), entry.name.length());
- hdr1.field_attach(field);
- hdr1.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- hdr2.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input2) {
- field = hdr2.field_create(entry.name.data(), entry.name.length());
- hdr2.field_attach(field);
- hdr2.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- HttpTransact::merge_response_header_with_cached_header(&hdr1, &hdr2);
-
- CHECK(hdr1.fields_count() == 5);
-
- field = hdr1.field_find("AAA", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "111", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("BBB", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "555", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("CCC", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "333", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("DDD", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "444", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("FFF", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "666", len) == 0);
- CHECK(field->has_dups() == false);
- }
-
- SECTION("Have dup headers")
- {
- HTTPHdr hdr1;
- HTTPHdr hdr2;
- MIMEField *field;
- const char *str;
- int len;
-
- struct header {
- std::string_view name;
- std::string_view value;
- };
-
- struct header input1[] = {
- {"AAA", "111"},
- {"BBB", "222"},
- {"CCC", "333"},
- };
- struct header input2[] = {
- {"DDD", "444"},
- {"EEE", "555"},
- {"EEE", "666"}
- };
-
- hdr1.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input1) {
- field = hdr1.field_create(entry.name.data(), entry.name.length());
- hdr1.field_attach(field);
- hdr1.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- hdr2.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input2) {
- field = hdr2.field_create(entry.name.data(), entry.name.length());
- hdr2.field_attach(field);
- hdr2.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- HttpTransact::merge_response_header_with_cached_header(&hdr1, &hdr2);
-
- CHECK(hdr1.fields_count() == 6);
-
- field = hdr1.field_find("AAA", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "111", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("BBB", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "222", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("CCC", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "333", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("DDD", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "444", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("EEE", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "555", len) == 0);
- CHECK(field->has_dups() == true);
- }
-
- SECTION("Have dup headers 2")
- {
- HTTPHdr hdr1;
- HTTPHdr hdr2;
- MIMEField *field;
- const char *str;
- int len;
-
- struct header {
- std::string_view name;
- std::string_view value;
- };
-
- struct header input1[] = {
- {"AAA", "111"},
- {"BBB", "222"},
- {"CCC", "333"},
- };
- struct header input2[] = {
- {"DDD", "444"},
- {"DDD", "555"},
- {"FFF", "666"}
- };
-
- hdr1.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input1) {
- field = hdr1.field_create(entry.name.data(), entry.name.length());
- hdr1.field_attach(field);
- hdr1.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- hdr2.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input2) {
- field = hdr2.field_create(entry.name.data(), entry.name.length());
- hdr2.field_attach(field);
- hdr2.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- HttpTransact::merge_response_header_with_cached_header(&hdr1, &hdr2);
-
- CHECK(hdr1.fields_count() == 6);
-
- field = hdr1.field_find("AAA", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "111", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("BBB", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "222", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("CCC", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "333", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("DDD", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "444", len) == 0);
- CHECK(field->has_dups() == true);
-
- field = hdr1.field_find("FFF", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "666", len) == 0);
- CHECK(field->has_dups() == false);
- }
-
- SECTION("Have common and dup headers")
- {
- HTTPHdr hdr1;
- HTTPHdr hdr2;
- MIMEField *field;
- const char *str;
- int len;
-
- struct header {
- std::string_view name;
- std::string_view value;
- };
-
- struct header input1[] = {
- {"AAA", "111"},
- {"BBB", "222"},
- {"CCC", "333"},
- {"DDD", "444"},
- };
- struct header input2[] = {
- {"AAA", "555"},
- {"BBB", "666"},
- {"BBB", "777"},
- {"CCC", "888"},
- {"EEE", "999"},
- };
-
- hdr1.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input1) {
- field = hdr1.field_create(entry.name.data(), entry.name.length());
- hdr1.field_attach(field);
- hdr1.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- hdr2.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : input2) {
- field = hdr2.field_create(entry.name.data(), entry.name.length());
- hdr2.field_attach(field);
- hdr2.field_value_set(field, entry.value.data(), entry.value.length());
- }
-
- HttpTransact::merge_response_header_with_cached_header(&hdr1, &hdr2);
-
- CHECK(hdr1.fields_count() == 6);
-
- field = hdr1.field_find("AAA", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "555", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("BBB", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "666", len) == 0);
- CHECK(field->has_dups() == true);
-
- ///////////// Dup //////////////////////////
- field = field->m_next_dup;
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "777", len) == 0);
- CHECK(field->has_dups() == false);
- ///////////////////////////////////////
-
- field = hdr1.field_find("CCC", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "888", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("DDD", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "444", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = hdr1.field_find("EEE", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "999", len) == 0);
- CHECK(field->has_dups() == false);
- }
- SECTION("Response has superset")
- {
- HTTPHdr cached_headers;
- HTTPHdr response_headers;
- MIMEField *field;
- const char *str;
- int len;
-
- struct header {
- std::string_view name;
- std::string_view value;
- };
-
- struct header cached[] = {
- {"Foo", "111"},
- {"Fizz", "555"},
- {"Bar", "333"},
- {"Bop", "666"},
- {"Bar", "222"},
- {"X-Foo", "aaa"},
- {"Eat", "444"},
- };
- // Response headers in a 304 should, in theory, match the cached
headers, but, what if they don't?
- // The response headers should still be merged into the cached object
properly given the existing logic.
- // In the following, the ordering is different from the cached headers,
the Bar headers are missing, and two duplicate Zip
- // headers are not in the cached object.
- struct header response[] = {
- {"X-Foo", "aaa"},
- {"Zip", "888"},
- {"Zip", "999"},
- {"Eat", "444"},
- {"Foo", "111"},
- {"Fizz", "555"},
- {"Bop", "666"},
- };
-
- cached_headers.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : cached) {
- field = cached_headers.field_create(entry.name.data(),
entry.name.length());
- cached_headers.field_attach(field);
- cached_headers.field_value_set(field, entry.value.data(),
entry.value.length());
- }
-
- response_headers.create(HTTP_TYPE_RESPONSE);
- for (auto &&entry : response) {
- field = response_headers.field_create(entry.name.data(),
entry.name.length());
- response_headers.field_attach(field);
- response_headers.field_value_set(field, entry.value.data(),
entry.value.length());
- }
-
- HttpTransact::merge_response_header_with_cached_header(&cached_headers,
&response_headers);
-
- CHECK(cached_headers.fields_count() == 9);
- CHECK(response_headers.fields_count() == 7);
-
- field = cached_headers.field_find("Foo", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "111", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = cached_headers.field_find("Fizz", 4);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "555", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = cached_headers.field_find("Bop", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "666", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = cached_headers.field_find("X-Foo", 5);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "aaa", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = cached_headers.field_find("Eat", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "444", len) == 0);
- CHECK(field->has_dups() == false);
-
- field = cached_headers.field_find("Bar", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "333", len) == 0);
- CHECK(field->has_dups() == true);
-
- ///////////// Dup //////////////////////////
- field = field->m_next_dup;
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "222", len) == 0);
- CHECK(field->has_dups() == false);
- ///////////////////////////////////////
-
- field = cached_headers.field_find("Zip", 3);
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "888", len) == 0);
- CHECK(field->has_dups() == true);
-
- ///////////// Dup //////////////////////////
- REQUIRE(field->m_next_dup != nullptr);
- field = field->m_next_dup;
- REQUIRE(field != nullptr);
- str = field->value_get(&len);
- CHECK(len == 3);
- CHECK(strncmp(str, "999", len) == 0);
- CHECK(field->has_dups() == false);
- ///////////////////////////////////////
- }
- }
-}
-
-#include "HttpSessionManager.h"
-HttpSessionManager httpSessionManager;
-StatPagesManager statPagesManager;
\ No newline at end of file
