diff --git a/src/proxy/hdrs/MIME.cc b/src/proxy/hdrs/MIME.cc index 8eae5d78529..5f2a089c4bd 100644 --- a/src/proxy/hdrs/MIME.cc +++ b/src/proxy/hdrs/MIME.cc @@ -3752,6 +3752,9 @@ MIMEHdrImpl::recompute_cooked_stuff(MIMEField *changing_field_or_null) for (s = csv_iter.get_first(field, &len); s != nullptr; s = csv_iter.get_next(&len)) { e = s + len; + // Store set mask bits from this CSV value so we can clear them if needed. + uint32_t csv_value_mask = 0; + for (c = s; (c < e) && (ParseRules::is_token(*c)); c++) { ; } @@ -3766,6 +3769,7 @@ MIMEHdrImpl::recompute_cooked_stuff(MIMEField *changing_field_or_null) HdrTokenHeapPrefix *p = hdrtoken_wks_to_prefix(token_wks); mask = p->wks_type_specific.u.cache_control.cc_mask; m_cooked_stuff.m_cache_control.m_mask |= mask; + csv_value_mask |= mask; #if TRACK_COOKING Dbg(dbg_ctl_http, " set mask 0x%0X", mask); @@ -3774,27 +3778,72 @@ MIMEHdrImpl::recompute_cooked_stuff(MIMEField *changing_field_or_null) if (mask & (MIME_COOKED_MASK_CC_MAX_AGE | MIME_COOKED_MASK_CC_S_MAXAGE | MIME_COOKED_MASK_CC_MAX_STALE | MIME_COOKED_MASK_CC_MIN_FRESH)) { int value; + // Per RFC 7230 Section 3.2.3, there should be no whitespace around '='. + const char *value_start = c; + + // Check if the next character is '=' (no space allowed before '='). + if (c < e && *c == '=') { + ++c; // Move past the '=' + + // Again: no whitespace after the '=' either. Keep in mind that values can be negative. + bool valid_syntax = (c < e) && (is_digit(*c) || *c == '-'); - if (mime_parse_integer(c, e, &value)) { + if (valid_syntax) { + // Reset to value_start to let mime_parse_integer do its work. + c = value_start; + if (mime_parse_integer(c, e, &value)) { #if TRACK_COOKING - Dbg(dbg_ctl_http, " set integer value %d", value); + Dbg(dbg_ctl_http, " set integer value %d", value); #endif - if (token_wks == MIME_VALUE_MAX_AGE.c_str()) { - m_cooked_stuff.m_cache_control.m_secs_max_age = value; - } else if (token_wks == MIME_VALUE_MIN_FRESH.c_str()) { - m_cooked_stuff.m_cache_control.m_secs_min_fresh = value; - } else if (token_wks == MIME_VALUE_MAX_STALE.c_str()) { - m_cooked_stuff.m_cache_control.m_secs_max_stale = value; - } else if (token_wks == MIME_VALUE_S_MAXAGE.c_str()) { - m_cooked_stuff.m_cache_control.m_secs_s_maxage = value; - } - } else { + if (token_wks == MIME_VALUE_MAX_AGE.c_str()) { + m_cooked_stuff.m_cache_control.m_secs_max_age = value; + } else if (token_wks == MIME_VALUE_MIN_FRESH.c_str()) { + m_cooked_stuff.m_cache_control.m_secs_min_fresh = value; + } else if (token_wks == MIME_VALUE_MAX_STALE.c_str()) { + m_cooked_stuff.m_cache_control.m_secs_max_stale = value; + } else if (token_wks == MIME_VALUE_S_MAXAGE.c_str()) { + m_cooked_stuff.m_cache_control.m_secs_s_maxage = value; + } + } else { #if TRACK_COOKING - Dbg(dbg_ctl_http, " set integer value %d", INT_MAX); + Dbg(dbg_ctl_http, " set integer value %d", INT_MAX); #endif - if (token_wks == MIME_VALUE_MAX_STALE.c_str()) { - m_cooked_stuff.m_cache_control.m_secs_max_stale = INT_MAX; + if (token_wks == MIME_VALUE_MAX_STALE.c_str()) { + m_cooked_stuff.m_cache_control.m_secs_max_stale = INT_MAX; + } + } + } else { + // Syntax is malformed (e.g., whitespace after '=', quotes around value, or no value). + // Treat this as unrecognized and clear the mask. + csv_value_mask = 0; + m_cooked_stuff.m_cache_control.m_mask &= ~mask; } + } else { + // No '=' found, or whitespace before '='. This is malformed. + // For directives that require values, this is an error. + // Clear the mask for this directive. + csv_value_mask = 0; + m_cooked_stuff.m_cache_control.m_mask &= ~mask; + } + } + + // Detect whether there is any more non-whitespace content after the + // directive. This indicates an unrecognized or malformed directive. + // This can happen, for instance, if the host uses semicolons + // instead of commas as separators which is against RFC 7234 (see + // issue #12029). Regardless of the cause, this means we need to + // ignore the directive and clear any mask bits we set from it. + while (c < e && ParseRules::is_ws(*c)) { + ++c; + } + if (c < e) { + // There's non-whitespace content that wasn't parsed. This means + // that we cannot really understand what this directive is. + // Per RFC 7234 Section 5.2: "A cache MUST ignore unrecognized cache + // directives." + if (csv_value_mask != 0) { + // Reverse the mask that we set above. + m_cooked_stuff.m_cache_control.m_mask &= ~csv_value_mask; } } } diff --git a/src/proxy/hdrs/unit_tests/test_HdrUtils.cc b/src/proxy/hdrs/unit_tests/test_HdrUtils.cc index 119a4bb4958..befd37b5502 100644 --- a/src/proxy/hdrs/unit_tests/test_HdrUtils.cc +++ b/src/proxy/hdrs/unit_tests/test_HdrUtils.cc @@ -24,33 +24,110 @@ #include #include #include +#include #include +#include +#include #include "proxy/hdrs/HdrHeap.h" #include "proxy/hdrs/MIME.h" #include "proxy/hdrs/HdrUtils.h" -TEST_CASE("HdrUtils", "[proxy][hdrutils]") +// Parameterized test for HdrCsvIter parsing. +TEST_CASE("HdrCsvIter", "[proxy][hdrutils]") { - static constexpr swoc::TextView text{"One: alpha\r\n" - "Two: alpha, bravo\r\n" - "Three: zwoop, \"A,B\" , , phil , \"unterminated\r\n" - "Five: alpha, bravo, charlie\r\n" - "Four: itchi, \"ni, \\\"san\" , \"\" , \"\r\n" - "Five: delta, echo\r\n" - "\r\n"}; - - static constexpr std::string_view ONE_TAG{"One"}; - static constexpr std::string_view TWO_TAG{"Two"}; - static constexpr std::string_view THREE_TAG{"Three"}; - static constexpr std::string_view FOUR_TAG{"Four"}; - static constexpr std::string_view FIVE_TAG{"Five"}; + constexpr bool COMBINE_DUPLICATES = true; + + // Structure for parameterized HdrCsvIter tests. + struct CsvIterTestCase { + const char *description; + const char *header_text; + const char *field_name; + std::vector expected_values; + bool combine_dups; // Parameter for get_first() + }; + + // Test cases for HdrCsvIter parsing. + // clang-format off + static const std::vector csv_iter_test_cases = { + // Basic CSV parsing tests + {"single value", + "One: alpha\r\n\r\n", + "One", + {"alpha"}, + COMBINE_DUPLICATES}, + + {"two values", + "Two: alpha, bravo\r\n\r\n", + "Two", + {"alpha", "bravo"}, + COMBINE_DUPLICATES}, + + {"quoted values and escaping", + "Three: zwoop, \"A,B\" , , phil , \"unterminated\r\n\r\n", + "Three", + {"zwoop", "A,B", "phil", "unterminated"}, + COMBINE_DUPLICATES}, + + {"escaped quotes passed through", + "Four: itchi, \"ni, \\\"san\" , \"\" , \"\r\n\r\n", + "Four", + {"itchi", "ni, \\\"san"}, + COMBINE_DUPLICATES}, + + {"duplicate fields combined", + "Five: alpha, bravo, charlie\r\nFive: delta, echo\r\n\r\n", + "Five", + {"alpha", "bravo", "charlie", "delta", "echo"}, + COMBINE_DUPLICATES}, + + {"duplicate fields not combined", + "Five: alpha, bravo, charlie\r\nFive: delta, echo\r\n\r\n", + "Five", + {"alpha", "bravo", "charlie"}, + !COMBINE_DUPLICATES}, + + // Cache-Control specific tests + {"Cache-Control: basic max-age and public", + "Cache-Control: max-age=30, public\r\n\r\n", + "Cache-Control", + {"max-age=30", "public"}, + COMBINE_DUPLICATES}, + + {"Cache-Control: extension directives with values", + "Cache-Control: stale-if-error=1, stale-while-revalidate=60, no-cache\r\n\r\n", + "Cache-Control", + {"stale-if-error=1", "stale-while-revalidate=60", "no-cache"}, + COMBINE_DUPLICATES}, + + {"Cache-Control: mixed directives", + "Cache-Control: public, max-age=300, s-maxage=600\r\n\r\n", + "Cache-Control", + {"public", "max-age=300", "s-maxage=600"}, + COMBINE_DUPLICATES}, + + {"Cache-Control: semicolon separator treated as single value", + "Cache-Control: public; max-age=30\r\n\r\n", + "Cache-Control", + {"public; max-age=30"}, + COMBINE_DUPLICATES}, + + {"Cache-Control: empty value", + "Cache-Control: \r\n\r\n", + "Cache-Control", + {}, + COMBINE_DUPLICATES}, + }; + // clang-format on + auto test_case = GENERATE(from_range(csv_iter_test_cases)); + + CAPTURE(test_case.description, test_case.header_text); HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); MIMEParser parser; - char const *real_s = text.data(); - char const *real_e = text.data_end(); + char const *real_s = test_case.header_text; + char const *real_e = test_case.header_text + strlen(test_case.header_text); MIMEHdr mime; mime.create(heap); @@ -60,65 +137,26 @@ TEST_CASE("HdrUtils", "[proxy][hdrutils]") REQUIRE(ParseResult::DONE == result); HdrCsvIter iter; - - MIMEField *field{mime.field_find(ONE_TAG)}; + MIMEField *field = mime.field_find(test_case.field_name); REQUIRE(field != nullptr); - auto value = iter.get_first(field); - REQUIRE(value == "alpha"); - - field = mime.field_find(TWO_TAG); - value = iter.get_first(field); - REQUIRE(value == "alpha"); - value = iter.get_next(); - REQUIRE(value == "bravo"); - value = iter.get_next(); - REQUIRE(value.empty()); - - field = mime.field_find(THREE_TAG); - value = iter.get_first(field); - REQUIRE(value == "zwoop"); - value = iter.get_next(); - REQUIRE(value == "A,B"); // quotes escape separator, and are stripped. - value = iter.get_next(); - REQUIRE(value == "phil"); - value = iter.get_next(); - REQUIRE(value == "unterminated"); - value = iter.get_next(); - REQUIRE(value.empty()); - - field = mime.field_find(FOUR_TAG); - value = iter.get_first(field); - REQUIRE(value == "itchi"); - value = iter.get_next(); - REQUIRE(value == "ni, \\\"san"); // verify escaped quotes are passed through. - value = iter.get_next(); - REQUIRE(value.empty()); - - // Check that duplicates are handled correctly. - field = mime.field_find(FIVE_TAG); - value = iter.get_first(field); - REQUIRE(value == "alpha"); - value = iter.get_next(); - REQUIRE(value == "bravo"); - value = iter.get_next(); - REQUIRE(value == "charlie"); - value = iter.get_next(); - REQUIRE(value == "delta"); - value = iter.get_next(); - REQUIRE(value == "echo"); - value = iter.get_next(); - REQUIRE(value.empty()); - - field = mime.field_find(FIVE_TAG); - value = iter.get_first(field, false); - REQUIRE(value == "alpha"); - value = iter.get_next(); - REQUIRE(value == "bravo"); - value = iter.get_next(); - REQUIRE(value == "charlie"); - value = iter.get_next(); - REQUIRE(value.empty()); + if (test_case.expected_values.empty()) { + auto value = iter.get_first(field, test_case.combine_dups); + REQUIRE(value.empty()); + } else { + auto value = iter.get_first(field, test_case.combine_dups); + REQUIRE(value == test_case.expected_values[0]); + + for (size_t i = 1; i < test_case.expected_values.size(); ++i) { + value = iter.get_next(); + REQUIRE(value == test_case.expected_values[i]); + } + + // After all expected values, the next should be empty. + value = iter.get_next(); + REQUIRE(value.empty()); + } + heap->destroy(); } @@ -207,3 +245,320 @@ TEST_CASE("HdrUtils 3", "[proxy][hdrutils]") REQUIRE(0 == memcmp(swoc::TextView(buff, idx), text)); heap->destroy(); }; + +// Test that malformed Cache-Control directives are properly ignored during cooking. +// All malformed directives should result in mask == 0. +TEST_CASE("Cache-Control Malformed Cooking", "[proxy][hdrutils]") +{ + struct MalformedCCTestCase { + const char *description; + const char *header_text; + }; + + // clang-format off + // These tests align with cache-tests.fyi/#cc-parse + static const std::vector malformed_cc_test_cases = { + // Separator issues + {"semicolon separator (should be comma)", + "Cache-Control: public; max-age=30\r\n\r\n"}, + + // Space around equals (cc-parse: max-age with space before/after =) + {"space before equals sign", + "Cache-Control: max-age =300\r\n\r\n"}, + + {"space after equals sign", + "Cache-Control: max-age= 300\r\n\r\n"}, + + {"space both before and after equals sign", + "Cache-Control: max-age = 300\r\n\r\n"}, + + // Quoted values (cc-parse: single-quoted max-age) + {"single quotes around value", + "Cache-Control: max-age='300'\r\n\r\n"}, + + {"double quotes around value", + "Cache-Control: max-age=\"300\"\r\n\r\n"}, + + // s-maxage variants + {"s-maxage with space before equals", + "Cache-Control: s-maxage =600\r\n\r\n"}, + + {"s-maxage with space after equals", + "Cache-Control: s-maxage= 600\r\n\r\n"}, + + // Invalid numeric values (cc-parse: decimal max-age) + {"decimal value in max-age (1.5)", + "Cache-Control: max-age=1.5\r\n\r\n"}, + + {"decimal value in max-age (3600.0)", + "Cache-Control: max-age=3600.0\r\n\r\n"}, + + {"decimal value starting with dot (.5)", + "Cache-Control: max-age=.5\r\n\r\n"}, + + {"decimal value in s-maxage", + "Cache-Control: s-maxage=1.5\r\n\r\n"}, + + // Leading and trailing alpha characters + {"leading alpha in max-age value", + "Cache-Control: max-age=a300\r\n\r\n"}, + + {"trailing alpha in max-age value", + "Cache-Control: max-age=300a\r\n\r\n"}, + + {"leading alpha in s-maxage value", + "Cache-Control: s-maxage=a600\r\n\r\n"}, + + {"trailing alpha in s-maxage value", + "Cache-Control: s-maxage=600a\r\n\r\n"}, + + // Empty and missing values + {"empty max-age value alone", + "Cache-Control: max-age=\r\n\r\n"}, + }; + // clang-format on + + auto test_case = GENERATE(from_range(malformed_cc_test_cases)); + + CAPTURE(test_case.description, test_case.header_text); + + HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); + MIMEParser parser; + char const *real_s = test_case.header_text; + char const *real_e = test_case.header_text + strlen(test_case.header_text); + MIMEHdr mime; + + mime.create(heap); + mime_parser_init(&parser); + + auto result = mime_parser_parse(&parser, heap, mime.m_mime, &real_s, real_e, false, true, false); + REQUIRE(ParseResult::DONE == result); + + mime.m_mime->recompute_cooked_stuff(); + + // All malformed directives should result in mask == 0. + auto mask = mime.get_cooked_cc_mask(); + REQUIRE(mask == 0); + + heap->destroy(); +} + +// Test that properly formed Cache-Control directives are correctly cooked. +TEST_CASE("Cache-Control Valid Cooking", "[proxy][hdrutils]") +{ + struct ValidCCTestCase { + const char *description; + const char *header_text; + uint32_t expected_mask; + int32_t expected_max_age; + int32_t expected_s_maxage; + int32_t expected_max_stale; + int32_t expected_min_fresh; + }; + + // Use 0 to indicate "don't care" for integer values (mask determines which are valid). + // clang-format off + static const std::vector valid_cc_test_cases = { + // Basic directives without values + {"public only", + "Cache-Control: public\r\n\r\n", + MIME_COOKED_MASK_CC_PUBLIC, + 0, 0, 0, 0}, + + {"private only", + "Cache-Control: private\r\n\r\n", + MIME_COOKED_MASK_CC_PRIVATE, + 0, 0, 0, 0}, + + {"no-cache only", + "Cache-Control: no-cache\r\n\r\n", + MIME_COOKED_MASK_CC_NO_CACHE, + 0, 0, 0, 0}, + + {"no-store only", + "Cache-Control: no-store\r\n\r\n", + MIME_COOKED_MASK_CC_NO_STORE, + 0, 0, 0, 0}, + + {"no-transform only", + "Cache-Control: no-transform\r\n\r\n", + MIME_COOKED_MASK_CC_NO_TRANSFORM, + 0, 0, 0, 0}, + + {"must-revalidate only", + "Cache-Control: must-revalidate\r\n\r\n", + MIME_COOKED_MASK_CC_MUST_REVALIDATE, + 0, 0, 0, 0}, + + {"proxy-revalidate only", + "Cache-Control: proxy-revalidate\r\n\r\n", + MIME_COOKED_MASK_CC_PROXY_REVALIDATE, + 0, 0, 0, 0}, + + {"only-if-cached only", + "Cache-Control: only-if-cached\r\n\r\n", + MIME_COOKED_MASK_CC_ONLY_IF_CACHED, + 0, 0, 0, 0}, + + // Directives with values + {"max-age=0", + "Cache-Control: max-age=0\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + 0, 0, 0, 0}, + + {"max-age=300", + "Cache-Control: max-age=300\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + 300, 0, 0, 0}, + + {"max-age=86400", + "Cache-Control: max-age=86400\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + 86400, 0, 0, 0}, + + {"s-maxage=600", + "Cache-Control: s-maxage=600\r\n\r\n", + MIME_COOKED_MASK_CC_S_MAXAGE, + 0, 600, 0, 0}, + + {"max-stale=100", + "Cache-Control: max-stale=100\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_STALE, + 0, 0, 100, 0}, + + {"min-fresh=60", + "Cache-Control: min-fresh=60\r\n\r\n", + MIME_COOKED_MASK_CC_MIN_FRESH, + 0, 0, 0, 60}, + + // Multiple directives + {"max-age and public", + "Cache-Control: max-age=300, public\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE | MIME_COOKED_MASK_CC_PUBLIC, + 300, 0, 0, 0}, + + {"public and max-age (reversed order)", + "Cache-Control: public, max-age=300\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE | MIME_COOKED_MASK_CC_PUBLIC, + 300, 0, 0, 0}, + + {"max-age and s-maxage", + "Cache-Control: max-age=300, s-maxage=600\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE | MIME_COOKED_MASK_CC_S_MAXAGE, + 300, 600, 0, 0}, + + {"private and no-cache", + "Cache-Control: private, no-cache\r\n\r\n", + MIME_COOKED_MASK_CC_PRIVATE | MIME_COOKED_MASK_CC_NO_CACHE, + 0, 0, 0, 0}, + + {"no-store and no-cache", + "Cache-Control: no-store, no-cache\r\n\r\n", + MIME_COOKED_MASK_CC_NO_STORE | MIME_COOKED_MASK_CC_NO_CACHE, + 0, 0, 0, 0}, + + {"must-revalidate and proxy-revalidate", + "Cache-Control: must-revalidate, proxy-revalidate\r\n\r\n", + MIME_COOKED_MASK_CC_MUST_REVALIDATE | MIME_COOKED_MASK_CC_PROXY_REVALIDATE, + 0, 0, 0, 0}, + + {"complex: public, max-age, s-maxage, must-revalidate", + "Cache-Control: public, max-age=300, s-maxage=600, must-revalidate\r\n\r\n", + MIME_COOKED_MASK_CC_PUBLIC | MIME_COOKED_MASK_CC_MAX_AGE | + MIME_COOKED_MASK_CC_S_MAXAGE | MIME_COOKED_MASK_CC_MUST_REVALIDATE, + 300, 600, 0, 0}, + + {"all request directives: max-age, max-stale, min-fresh, no-cache, no-store, no-transform, only-if-cached", + "Cache-Control: max-age=100, max-stale=200, min-fresh=50, no-cache, no-store, no-transform, only-if-cached\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE | MIME_COOKED_MASK_CC_MAX_STALE | MIME_COOKED_MASK_CC_MIN_FRESH | + MIME_COOKED_MASK_CC_NO_CACHE | MIME_COOKED_MASK_CC_NO_STORE | + MIME_COOKED_MASK_CC_NO_TRANSFORM | MIME_COOKED_MASK_CC_ONLY_IF_CACHED, + 100, 0, 200, 50}, + + // Edge cases - whitespace + {"extra whitespace around directive", + "Cache-Control: max-age=300 \r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + 300, 0, 0, 0}, + + {"extra whitespace between directives", + "Cache-Control: max-age=300 , public\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE | MIME_COOKED_MASK_CC_PUBLIC, + 300, 0, 0, 0}, + + {"tab character in header value", + "Cache-Control:\tmax-age=300\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + 300, 0, 0, 0}, + + // Edge cases - unknown directives + {"unknown directive ignored, known directive parsed", + "Cache-Control: unknown-directive, max-age=300\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + 300, 0, 0, 0}, + + {"unknown directive with value ignored", + "Cache-Control: unknown=value, public\r\n\r\n", + MIME_COOKED_MASK_CC_PUBLIC, + 0, 0, 0, 0}, + + // Edge cases - numeric values (cc-parse: 0000 max-age, large max-age) + {"max-age with leading zeros (cc-parse: 0000 max-age)", + "Cache-Control: max-age=0000\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + 0, 0, 0, 0}, + + {"max-age with leading zeros and value", + "Cache-Control: max-age=00300\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + 300, 0, 0, 0}, + + {"large max-age value", + "Cache-Control: max-age=999999999\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + 999999999, 0, 0, 0}, + + // Edge cases - negative values should be parsed (behavior per implementation) + {"negative max-age value", + "Cache-Control: max-age=-1\r\n\r\n", + MIME_COOKED_MASK_CC_MAX_AGE, + -1, 0, 0, 0}, + }; + // clang-format on + + auto test_case = GENERATE(from_range(valid_cc_test_cases)); + + CAPTURE(test_case.description, test_case.header_text); + + HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); + MIMEParser parser; + char const *real_s = test_case.header_text; + char const *real_e = test_case.header_text + strlen(test_case.header_text); + MIMEHdr mime; + + mime.create(heap); + mime_parser_init(&parser); + + auto result = mime_parser_parse(&parser, heap, mime.m_mime, &real_s, real_e, false, true, false); + REQUIRE(ParseResult::DONE == result); + + mime.m_mime->recompute_cooked_stuff(); + + auto mask = mime.get_cooked_cc_mask(); + REQUIRE(mask == test_case.expected_mask); + + if (test_case.expected_mask & MIME_COOKED_MASK_CC_MAX_AGE) { + REQUIRE(mime.get_cooked_cc_max_age() == test_case.expected_max_age); + } + if (test_case.expected_mask & MIME_COOKED_MASK_CC_S_MAXAGE) { + REQUIRE(mime.get_cooked_cc_s_maxage() == test_case.expected_s_maxage); + } + if (test_case.expected_mask & MIME_COOKED_MASK_CC_MAX_STALE) { + REQUIRE(mime.get_cooked_cc_max_stale() == test_case.expected_max_stale); + } + if (test_case.expected_mask & MIME_COOKED_MASK_CC_MIN_FRESH) { + REQUIRE(mime.get_cooked_cc_min_fresh() == test_case.expected_min_fresh); + } + + heap->destroy(); +} diff --git a/tests/gold_tests/cache/negative-caching.test.py b/tests/gold_tests/cache/negative-caching.test.py index 0e09ae17bb4..853624e6348 100644 --- a/tests/gold_tests/cache/negative-caching.test.py +++ b/tests/gold_tests/cache/negative-caching.test.py @@ -132,3 +132,6 @@ p.StartBefore(dns) p.StartBefore(server) p.StartBefore(ts) + +# Test malformed Cache-Control header with semicolons instead of commas (issue #12029) +Test.ATSReplayTest(replay_file="replay/negative-caching-malformed-cc.replay.yaml") diff --git a/tests/gold_tests/cache/replay/negative-caching-malformed-cc.replay.yaml b/tests/gold_tests/cache/replay/negative-caching-malformed-cc.replay.yaml new file mode 100644 index 00000000000..c973aa14b75 --- /dev/null +++ b/tests/gold_tests/cache/replay/negative-caching-malformed-cc.replay.yaml @@ -0,0 +1,317 @@ +# 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. + +# +# This replay file tests negative caching with malformed Cache-Control headers +# that use semicolons instead of commas as separators (issue #12029). +# + +meta: + version: "1.0" + +# Configuration section for autest integration +autest: + description: 'Test malformed Cache-Control header with semicolons instead of commas (issue #12029)' + + # Server configuration + server: + name: 'server-malformed-cc' + + # Client configuration + client: + name: 'client-malformed-cc' + + # ATS configuration + ats: + name: 'ts-malformed-cc' + + # ATS records.config settings + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'http' + proxy.config.http.insert_age_in_response: 0 + proxy.config.http.negative_caching_enabled: 0 + + # Remap configuration + remap_config: + - from: "/" + to: "http://127.0.0.1:{SERVER_HTTP_PORT}/" + +sessions: +- transactions: + + # + # Phase 1: Initial requests to populate the cache. + # + # These requests are sent to the origin server and the responses are cached + # (or not cached, depending on the Cache-Control header). + # + + # First, verify that a 400 response with a proper Cache-Control header is cached. + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_proper_cc + headers: + fields: + - [ Host, example.com ] + - [ uuid, proper_cc ] + + server-response: + status: 400 + reason: "Bad Request" + headers: + fields: + - [ Content-Length, 0 ] + - [ Cache-Control, max-age=300 ] + + proxy-response: + status: 400 + + # Test: Verify that a 400 response with malformed Cache-Control using + # semicolons is not cached. The header "Cache-Control: public; max-age=30" + # uses semicolons instead of commas as separators, violating RFC 7234. + # Per RFC 7234 Section 5.2, caches must ignore unrecognized directives. + # Since the directive is malformed, it should be ignored, and the response + # should not be cached (no valid Cache-Control + negative_caching_enabled=0). + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_malformed_cc_semicolon + headers: + fields: + - [ Host, example.com ] + - [ uuid, semicolon ] + + server-response: + status: 400 + reason: "Bad Request" + headers: + fields: + - [ Content-Length, 0 ] + # Note: Using semicolon instead of comma - this is malformed per RFC 7234 + - [ Cache-Control, "public; max-age=30" ] + + proxy-response: + status: 400 + + # Test: Verify that a 400 response with space before = in max-age is not cached. + # The header "Cache-Control: max-age =300" is malformed per RFC 7230. + # Per RFC 7234 Section 5.2, caches must ignore unrecognized directives. + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_malformed_cc_space_before_equals + headers: + fields: + - [ Host, example.com ] + - [ uuid, space_before_equals ] + + server-response: + status: 400 + reason: "Bad Request" + headers: + fields: + - [ Content-Length, 0 ] + # Note: Space before = is malformed per RFC 7230 + - [ Cache-Control, "max-age =300" ] + + proxy-response: + status: 400 + + # Test: Verify that a 400 response with space after = in max-age is not cached. + # The header "Cache-Control: max-age= 300" is malformed per RFC 7230. + # Per RFC 7234 Section 5.2, caches must ignore unrecognized directives. + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_malformed_cc_space_after_equals + headers: + fields: + - [ Host, example.com ] + - [ uuid, space_after_equals ] + + server-response: + status: 400 + reason: "Bad Request" + headers: + fields: + - [ Content-Length, 0 ] + # Note: Space after = is malformed per RFC 7230 + - [ Cache-Control, "max-age= 300" ] + + proxy-response: + status: 400 + + # Test: Verify that a 400 response with single-quoted max-age value is not cached. + # The header "Cache-Control: max-age='300'" is malformed per RFC 7230. + # Per RFC 7234 Section 5.2, caches must ignore unrecognized directives. + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_malformed_cc_single_quotes + headers: + fields: + - [ Host, example.com ] + - [ uuid, single_quotes ] + + server-response: + status: 400 + reason: "Bad Request" + headers: + fields: + - [ Content-Length, 0 ] + # Note: Single quotes around value are malformed per RFC 7230 + - [ Cache-Control, "max-age='300'" ] + + proxy-response: + status: 400 + + # + # Phase 2: Verification requests to check if responses were cached. + # + # These requests should be served from the cache (if cached) or from the + # origin server (if not cached). + # + + # Second request to /path/400_proper_cc should be served from cache. + - client-request: + + # Add delay to ensure time for cache IO processing. + delay: 100ms + + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_proper_cc + headers: + fields: + - [ Host, example.com ] + - [ uuid, proper_cc_verify ] + + # The server should not receive this request because it should be served from cache. + server-response: + status: 200 + reason: "OK" + headers: + fields: + - [ Content-Length, 0 ] + - [ Cache-Control, max-age=300 ] + + proxy-response: + status: 400 + + # Second request to /path/400_malformed_cc_semicolon should NOT be served from cache. + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_malformed_cc_semicolon + headers: + fields: + - [ Host, example.com ] + - [ uuid, semicolon_verify ] + + # Since the initial CC was malformed, the response should not be cached. + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, 16 ] + - [ Cache-Control, max-age=300 ] + + # Expect the origin's 200 response. + proxy-response: + status: 200 + + # Second request to /path/400_malformed_cc_space_before_equals should NOT be served from cache. + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_malformed_cc_space_before_equals + headers: + fields: + - [ Host, example.com ] + - [ uuid, space_before_equals_verify ] + + # Since the initial CC was malformed, the response should not be cached. + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, 16 ] + - [ Cache-Control, max-age=300 ] + + # Expect the origin's 200 response. + proxy-response: + status: 200 + + # Second request to /path/400_malformed_cc_space_after_equals should NOT be served from cache. + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_malformed_cc_space_after_equals + headers: + fields: + - [ Host, example.com ] + - [ uuid, space_after_equals_verify ] + + # Since the initial CC was malformed, the response should not be cached. + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, 16 ] + - [ Cache-Control, max-age=300 ] + + # Expect the origin's 200 response. + proxy-response: + status: 200 + + # Second request to /path/400_malformed_cc_single_quotes should NOT be served from cache. + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/400_malformed_cc_single_quotes + headers: + fields: + - [ Host, example.com ] + - [ uuid, single_quotes_verify ] + + # Since the initial CC was malformed, the response should not be cached. + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, 16 ] + - [ Cache-Control, max-age=300 ] + + # Expect the origin's 200 response. + proxy-response: + status: 200 + diff --git a/tests/gold_tests/pluginTest/stale_response/stale_response_no_default.replay.yaml b/tests/gold_tests/pluginTest/stale_response/stale_response_no_default.replay.yaml index 35d72c5973e..d0961053b55 100644 --- a/tests/gold_tests/pluginTest/stale_response/stale_response_no_default.replay.yaml +++ b/tests/gold_tests/pluginTest/stale_response/stale_response_no_default.replay.yaml @@ -135,7 +135,7 @@ sessions: - [ Content-Type, image/jpeg ] - [ Content-Length, 100 ] - [ Connection, keep-alive ] - - [ Cache-Control, max-age=1 stale-if-error=30 ] + - [ Cache-Control, "max-age=1, stale-if-error=30" ] - [ X-Response, fourth-response ] # We better have gone back to the origin and gotten second-response. diff --git a/tests/gold_tests/pluginTest/stale_response/stale_response_with_force_sie.replay.yaml b/tests/gold_tests/pluginTest/stale_response/stale_response_with_force_sie.replay.yaml index 10a8cd68f2b..5837d68384f 100644 --- a/tests/gold_tests/pluginTest/stale_response/stale_response_with_force_sie.replay.yaml +++ b/tests/gold_tests/pluginTest/stale_response/stale_response_with_force_sie.replay.yaml @@ -43,7 +43,7 @@ sessions: - [ Content-Length, 100 ] - [ Connection, keep-alive ] # Configure a small stale-if-error. - - [ Cache-Control, max-age=1 stale-if-error=1 ] + - [ Cache-Control, "max-age=1, stale-if-error=1" ] - [ X-Response, first-response ] proxy-response: diff --git a/tests/gold_tests/pluginTest/stale_response/stale_response_with_force_swr.replay.yaml b/tests/gold_tests/pluginTest/stale_response/stale_response_with_force_swr.replay.yaml index a980bb5010e..7a54ee94b36 100644 --- a/tests/gold_tests/pluginTest/stale_response/stale_response_with_force_swr.replay.yaml +++ b/tests/gold_tests/pluginTest/stale_response/stale_response_with_force_swr.replay.yaml @@ -42,14 +42,14 @@ sessions: - [ Connection, keep-alive ] # The low stale-while-revalidate should be overridden by # --force-stale-while-revalidate. - - [ Cache-Control, max-age=1 stale-while-revalidate=1 ] + - [ Cache-Control, "max-age=1, stale-while-revalidate=1" ] - [ X-Response, first-response ] proxy-response: status: 200 headers: fields: - - [ Cache-Control, { value: "max-age=1 stale-while-revalidate=1", as: equal } ] + - [ Cache-Control, { value: "max-age=1, stale-while-revalidate=1", as: equal } ] - [ X-Response, { value: first-response, as: equal } ] - client-request: