Skip to content

Commit f9aa1e0

Browse files
authored
Strictly follow Content-Length header ABNF rule (#10144)
- Replace content-length value type from const char * to std::string_view
1 parent ae190a5 commit f9aa1e0

File tree

2 files changed

+18
-13
lines changed

2 files changed

+18
-13
lines changed

proxy/hdrs/HTTP.cc

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <cassert>
2828
#include <cstdio>
2929
#include <cstring>
30+
#include <string_view>
3031
#include "HTTP.h"
3132
#include "HdrToken.h"
3233
#include "tscore/Diags.h"
@@ -1261,26 +1262,27 @@ validate_hdr_content_length(HdrHeap *heap, HTTPHdrImpl *hh)
12611262
// recipient MUST treat it as an unrecoverable error. If this is a
12621263
// request message, the server MUST respond with a 400 (Bad Request)
12631264
// status code and then close the connection
1264-
int content_length_len = 0;
1265-
const char *content_length_val = content_length_field->value_get(&content_length_len);
1265+
std::string_view value = content_length_field->value_get();
12661266

1267-
// RFC 7230 section 3.3.2
1267+
// RFC 9110 section 8.6.
12681268
// Content-Length = 1*DIGIT
12691269
//
1270+
if (value.empty()) {
1271+
Debug("http", "Content-Length headers don't match the ABNF, returning parse error");
1272+
return PARSE_RESULT_ERROR;
1273+
}
1274+
12701275
// If the content-length value contains a non-numeric value, the header is invalid
1271-
for (int i = 0; i < content_length_len; i++) {
1272-
if (!isdigit(content_length_val[i])) {
1273-
Debug("http", "Content-Length value contains non-digit, returning parse error");
1274-
return PARSE_RESULT_ERROR;
1275-
}
1276+
if (std::find_if(value.cbegin(), value.cend(), [](std::string_view::value_type c) { return !std::isdigit(c); }) !=
1277+
value.cend()) {
1278+
Debug("http", "Content-Length value contains non-digit, returning parse error");
1279+
return PARSE_RESULT_ERROR;
12761280
}
12771281

12781282
while (content_length_field->has_dups()) {
1279-
int content_length_len_2 = 0;
1280-
const char *content_length_val_2 = content_length_field->m_next_dup->value_get(&content_length_len_2);
1283+
std::string_view value_dup = content_length_field->m_next_dup->value_get();
12811284

1282-
if ((content_length_len != content_length_len_2) ||
1283-
(memcmp(content_length_val, content_length_val_2, content_length_len) != 0)) {
1285+
if ((value.length() != value_dup.length()) || value.compare(value_dup) != 0) {
12841286
// Values are different, parse error
12851287
Debug("http", "Content-Length headers don't match, returning parse error");
12861288
return PARSE_RESULT_ERROR;

proxy/hdrs/unit_tests/test_Hdrs.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ TEST_CASE("HdrTestHttpParse", "[proxy][hdrtest]")
4646
int expected_result;
4747
int expected_bytes_consumed;
4848
};
49-
static const std::array<Test, 23> tests = {
49+
static const std::array<Test, 26> tests = {
5050
{
5151
{"GET /index.html HTTP/1.0\r\n", PARSE_RESULT_DONE, 26},
5252
{"GET /index.html HTTP/1.0\r\n\r\n***BODY****", PARSE_RESULT_DONE, 28},
@@ -68,6 +68,9 @@ TEST_CASE("HdrTestHttpParse", "[proxy][hdrtest]")
6868
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n", PARSE_RESULT_DONE, 46},
6969
{"GET /index.html HTTP/1.0\r\n", PARSE_RESULT_DONE, 26},
7070
{"GET /index.html hTTP/1.0\r\n", PARSE_RESULT_ERROR, 26},
71+
{"POST /index.html HTTP/1.0\r\nContent-Length: 0\r\n\r\n", PARSE_RESULT_DONE, 48},
72+
{"POST /index.html HTTP/1.0\r\nContent-Length: \r\n\r\n", PARSE_RESULT_ERROR, 47},
73+
{"POST /index.html HTTP/1.0\r\nContent-Length:\r\n\r\n", PARSE_RESULT_ERROR, 46},
7174
{"CONNECT foo.example HTTP/1.1\r\n", PARSE_RESULT_DONE, 30},
7275
{"GET foo.example HTTP/1.1\r\n", PARSE_RESULT_ERROR, 26},
7376
{"", PARSE_RESULT_ERROR, 0},

0 commit comments

Comments
 (0)