From 71d8e9cefd86b9e225490bca5de28cb00bedd35d Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Wed, 26 May 2021 15:25:26 +0000 Subject: [PATCH 1/4] Enforce case for well known methods --- proxy/hdrs/HTTP.cc | 2 +- proxy/hdrs/HdrToken.cc | 24 +++++++++++++++++++ proxy/hdrs/HdrToken.h | 1 + tests/gold_tests/headers/gold/bad_method.gold | 24 +++++++++++++++++++ .../headers/good_request_after_bad.test.py | 10 ++++++++ 5 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 tests/gold_tests/headers/gold/bad_method.gold diff --git a/proxy/hdrs/HTTP.cc b/proxy/hdrs/HTTP.cc index 7b7baa62522..793f6e84b98 100644 --- a/proxy/hdrs/HTTP.cc +++ b/proxy/hdrs/HTTP.cc @@ -1098,7 +1098,7 @@ http_parser_parse_req(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const ink_assert(url_start); ink_assert(url_end); - int method_wks_idx = hdrtoken_tokenize(method_start, static_cast(method_end - method_start)); + int method_wks_idx = hdrtoken_method_tokenize(method_start, static_cast(method_end - method_start)); http_hdr_method_set(heap, hh, method_start, method_wks_idx, static_cast(method_end - method_start), must_copy_strings); ink_assert(hh->u.req.m_url_impl != nullptr); diff --git a/proxy/hdrs/HdrToken.cc b/proxy/hdrs/HdrToken.cc index 5fe2280988a..a4e161c4527 100644 --- a/proxy/hdrs/HdrToken.cc +++ b/proxy/hdrs/HdrToken.cc @@ -548,6 +548,30 @@ hdrtoken_tokenize_dfa(const char *string, int string_len, const char **wks_strin return wks_idx; } +/*------------------------------------------------------------------------- + Have to work around that methods are case insensitive while the DFA is + case insensitive. + -------------------------------------------------------------------------*/ + +int +hdrtoken_method_tokenize(const char *string, int string_len) +{ + const char *string_out; + int retval = -1; + if (hdrtoken_is_wks(string)) { + retval = hdrtoken_wks_to_index(string); + return retval; + } + retval = hdrtoken_tokenize(string, string_len, &string_out); + if (retval >= 0) { + if (strncmp(string, string_out, string_len) != 0) { + // Not a case match + retval = -1; + } + } + return retval; +} + /*------------------------------------------------------------------------- -------------------------------------------------------------------------*/ diff --git a/proxy/hdrs/HdrToken.h b/proxy/hdrs/HdrToken.h index dc3f34fe232..ba162b68af6 100644 --- a/proxy/hdrs/HdrToken.h +++ b/proxy/hdrs/HdrToken.h @@ -104,6 +104,7 @@ extern uint32_t hdrtoken_str_flags[]; extern void hdrtoken_init(); extern int hdrtoken_tokenize_dfa(const char *string, int string_len, const char **wks_string_out = nullptr); inkcoreapi extern int hdrtoken_tokenize(const char *string, int string_len, const char **wks_string_out = nullptr); +extern int hdrtoken_method_tokenize(const char *string, int string_len); extern const char *hdrtoken_string_to_wks(const char *string); extern const char *hdrtoken_string_to_wks(const char *string, int length); diff --git a/tests/gold_tests/headers/gold/bad_method.gold b/tests/gold_tests/headers/gold/bad_method.gold new file mode 100644 index 00000000000..3a9558b1cd3 --- /dev/null +++ b/tests/gold_tests/headers/gold/bad_method.gold @@ -0,0 +1,24 @@ +HTTP/1.1 501 Unsupported method ('gET') +Content-Type: text/html;charset=utf-8 +Content-Length: 496 +Date: `` +Age: 0 +Connection: keep-alive +Server: ATS/`` + + + + + + Error response + + +

Error response

+

Error code: 501

+

Message: Unsupported method ('gET').

+

Error code explanation: HTTPStatus.NOT_IMPLEMENTED - Server does not support this operation.

+ + +HTTP/1.1 200 OK +`` diff --git a/tests/gold_tests/headers/good_request_after_bad.test.py b/tests/gold_tests/headers/good_request_after_bad.test.py index 9a76975dbd7..be5deb166e9 100644 --- a/tests/gold_tests/headers/good_request_after_bad.test.py +++ b/tests/gold_tests/headers/good_request_after_bad.test.py @@ -27,6 +27,7 @@ ts.Disk.records_config.update({'proxy.config.diags.debug.tags': 'http', 'proxy.config.diags.debug.enabled': 0, +Test.ContinueOnFail = True }) server = Test.MakeOriginServer("server") @@ -91,3 +92,12 @@ tr.Processes.Default.Streams.All = Testers.ContainsExpression( r"HTTP/1.1 501 Unsupported method \('TRACE'\)", "microserver does not support TRACE") + +# Mixed case method name; Should fail but ATS is treating gET as GET +tr = Test.AddTestRun("mixed case method") +tr.Processes.Default.Command = 'printf "gET / HTTP/1.1\r\nHost:bob\r\n\r\nGET / HTTP/1.1\r\nHost: boa\r\n\r\n" | nc 127.0.0.1 {}'.format( + ts.Variables.port) +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = 'gold/bad_method.gold' + + From 2cb2d3cfdd3e3e353c2c3da190630b0487b2ca74 Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Fri, 28 May 2021 14:12:10 +0000 Subject: [PATCH 2/4] Fix comment --- tests/gold_tests/headers/good_request_after_bad.test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/gold_tests/headers/good_request_after_bad.test.py b/tests/gold_tests/headers/good_request_after_bad.test.py index be5deb166e9..d667af20f85 100644 --- a/tests/gold_tests/headers/good_request_after_bad.test.py +++ b/tests/gold_tests/headers/good_request_after_bad.test.py @@ -93,7 +93,7 @@ r"HTTP/1.1 501 Unsupported method \('TRACE'\)", "microserver does not support TRACE") -# Mixed case method name; Should fail but ATS is treating gET as GET +# Methods are case sensitive. Verify that "gET" is not confused with "GET". tr = Test.AddTestRun("mixed case method") tr.Processes.Default.Command = 'printf "gET / HTTP/1.1\r\nHost:bob\r\n\r\nGET / HTTP/1.1\r\nHost: boa\r\n\r\n" | nc 127.0.0.1 {}'.format( ts.Variables.port) From e0878d22d111159cb72d1a83fb0e95dd2a75384d Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Wed, 16 Jun 2021 19:57:34 +0000 Subject: [PATCH 3/4] make autopep happy --- tests/gold_tests/headers/good_request_after_bad.test.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/gold_tests/headers/good_request_after_bad.test.py b/tests/gold_tests/headers/good_request_after_bad.test.py index d667af20f85..73cc0cc60b4 100644 --- a/tests/gold_tests/headers/good_request_after_bad.test.py +++ b/tests/gold_tests/headers/good_request_after_bad.test.py @@ -27,7 +27,7 @@ ts.Disk.records_config.update({'proxy.config.diags.debug.tags': 'http', 'proxy.config.diags.debug.enabled': 0, -Test.ContinueOnFail = True + Test.ContinueOnFail=True }) server = Test.MakeOriginServer("server") @@ -99,5 +99,3 @@ ts.Variables.port) tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Streams.stdout = 'gold/bad_method.gold' - - From b1996e3daf07d0de3eb9c48f01945e84c2e6cc53 Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Wed, 23 Jun 2021 16:50:14 +0000 Subject: [PATCH 4/4] Fix merge error in test --- tests/gold_tests/headers/good_request_after_bad.test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/gold_tests/headers/good_request_after_bad.test.py b/tests/gold_tests/headers/good_request_after_bad.test.py index 73cc0cc60b4..27d4e65e457 100644 --- a/tests/gold_tests/headers/good_request_after_bad.test.py +++ b/tests/gold_tests/headers/good_request_after_bad.test.py @@ -27,9 +27,9 @@ ts.Disk.records_config.update({'proxy.config.diags.debug.tags': 'http', 'proxy.config.diags.debug.enabled': 0, - Test.ContinueOnFail=True }) +Test.ContinueOnFail = True server = Test.MakeOriginServer("server") request_header = {"headers": "GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""} response_header = {