diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 63dfd5d6dcb..bda05d7475a 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -5042,11 +5042,17 @@ 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) { - cached_header->field_delete(&*spot2, true); + MIMEField &field2{*spot2}; + name2 = field2.name_get(&name_len2); + + cached_header->field_delete(name2, name_len2); } dups_seen = true; } diff --git a/proxy/http/Makefile.am b/proxy/http/Makefile.am index df6d8468a8d..47447853aba 100644 --- a/proxy/http/Makefile.am +++ b/proxy/http/Makefile.am @@ -86,7 +86,7 @@ if BUILD_TESTS libhttp_a_SOURCES += RegressionHttpTransact.cc endif -check_PROGRAMS = test_proxy_http test_PreWarm +check_PROGRAMS = test_proxy_http test_PreWarm test_HttpTransact TESTS = $(check_PROGRAMS) @@ -122,6 +122,33 @@ 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 new file mode 100644 index 00000000000..334de97c410 --- /dev/null +++ b/proxy/http/unit_tests/main.cc @@ -0,0 +1,60 @@ +/** @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 new file mode 100644 index 00000000000..caa07e9e724 --- /dev/null +++ b/proxy/http/unit_tests/test_HttpTransact.cc @@ -0,0 +1,573 @@ +/** @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 +#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