Skip to content

Ensure a reason phrase when sending an HTTP/1 response #9615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bneradt merged 1 commit into from
Apr 19, 2023
Merged

Ensure a reason phrase when sending an HTTP/1 response #9615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions proxy/http/HttpTransact.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7834,7 +7834,19 @@ HttpTransact::build_response(State *s, HTTPHdr *base_response, HTTPHdr *outgoing
HTTPStatus status_code, const char *reason_phrase)
{
if (reason_phrase == nullptr) {
reason_phrase = http_hdr_reason_lookup(status_code);
if (status_code != HTTP_STATUS_NONE) {
reason_phrase = http_hdr_reason_lookup(status_code);
Debug("http_transact", "Using reason phrase from status %d: %s", status_code, reason_phrase);
Comment thread
bneradt marked this conversation as resolved.
} else if (base_response != nullptr && base_response->status_get() != HTTP_STATUS_NONE) {
HTTPStatus const base_response_status = base_response->status_get();
reason_phrase = http_hdr_reason_lookup(base_response_status);
Debug("http_transact", "Using reason phrase from base_response status %d: %s", base_response_status, reason_phrase);
} else {
// We have to set some value for build_base_response which expects a
// non-nullptr reason_phrase.
reason_phrase = http_hdr_reason_lookup(status_code);
Debug("http_transact", "Using HTTP_STATUS_NONE reason phrase %d: %s", status_code, reason_phrase);
}
}

if (base_response == nullptr) {
Expand Down Expand Up @@ -7942,7 +7954,15 @@ HttpTransact::build_response(State *s, HTTPHdr *base_response, HTTPHdr *outgoing
HttpTransactHeaders::insert_via_header_in_response(s, outgoing_response);
}

HttpTransactHeaders::convert_response(outgoing_version, outgoing_response);
// When converting a response, only set a reason phrase if one was not already
// set via some explicit call above.
char const *reason_phrase_for_convert = nullptr;
int outgoing_reason_phrase_len = 0;
char const *outgoing_reason_phrase = outgoing_response->reason_get(&outgoing_reason_phrase_len);
if (outgoing_reason_phrase == nullptr || outgoing_reason_phrase_len == 0) {
reason_phrase_for_convert = reason_phrase;
}
HttpTransactHeaders::convert_response(outgoing_version, outgoing_response, reason_phrase_for_convert);

// process reverse mappings on the location header
// TS-1364: do this regardless of response code
Expand Down
22 changes: 17 additions & 5 deletions proxy/http/HttpTransactHeaders.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,12 @@ HttpTransactHeaders::convert_request(HTTPVersion outgoing_ver, HTTPHdr *outgoing
////////////////////////////////////////////////////////////////////////
// Just convert the outgoing response to the appropriate version
void
HttpTransactHeaders::convert_response(HTTPVersion outgoing_ver, HTTPHdr *outgoing_response)
HttpTransactHeaders::convert_response(HTTPVersion outgoing_ver, HTTPHdr *outgoing_response, char const *reason_phrase)
{
if (outgoing_ver == HTTPVersion(1, 1)) {
convert_to_1_1_response_header(outgoing_response);
convert_to_1_1_response_header(outgoing_response, reason_phrase);
} else if (outgoing_ver == HTTPVersion(1, 0)) {
convert_to_1_0_response_header(outgoing_response);
convert_to_1_0_response_header(outgoing_response, reason_phrase);
} else {
Debug("http_trans", "[HttpTransactHeaders::convert_response]"
"Unsupported Version - passing through");
Expand Down Expand Up @@ -325,7 +325,7 @@ HttpTransactHeaders::convert_to_1_1_request_header(HTTPHdr *outgoing_request)
////////////////////////////////////////////////////////////////////////
// Take an existing outgoing response header and make it HTTP/1.0
void
HttpTransactHeaders::convert_to_1_0_response_header(HTTPHdr *outgoing_response)
HttpTransactHeaders::convert_to_1_0_response_header(HTTPHdr *outgoing_response, char const *reason_phrase)
{
// // These are required
// ink_assert(outgoing_response->status_get());
Expand All @@ -334,6 +334,12 @@ HttpTransactHeaders::convert_to_1_0_response_header(HTTPHdr *outgoing_response)
// Set HTTP version to 1.0
outgoing_response->version_set(HTTPVersion(1, 0));

// Set reason phrase if passed in.
if (reason_phrase != nullptr) {
Debug("http_transact_headers", "Setting HTTP/1.0 reason phrase to '%s'", reason_phrase);
outgoing_response->reason_set(reason_phrase, strlen(reason_phrase));
}

// Keep-Alive?

// Cache-Control?
Expand All @@ -342,13 +348,19 @@ HttpTransactHeaders::convert_to_1_0_response_header(HTTPHdr *outgoing_response)
////////////////////////////////////////////////////////////////////////
// Take an existing outgoing response header and make it HTTP/1.1
void
HttpTransactHeaders::convert_to_1_1_response_header(HTTPHdr *outgoing_response)
HttpTransactHeaders::convert_to_1_1_response_header(HTTPHdr *outgoing_response, char const *reason_phrase)
{
// These are required
ink_assert(outgoing_response->status_get());

// Set HTTP version to 1.1
outgoing_response->version_set(HTTPVersion(1, 1));

// Set reason phrase if passed in.
if (reason_phrase != nullptr) {
Debug("http_transact_headers", "Setting HTTP/1.1 reason phrase to '%s'", reason_phrase);
outgoing_response->reason_set(reason_phrase, strlen(reason_phrase));
}
}

///////////////////////////////////////////////////////////////////////////////
Expand Down
6 changes: 3 additions & 3 deletions proxy/http/HttpTransactHeaders.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ class HttpTransactHeaders
static void copy_header_fields(HTTPHdr *src_hdr, HTTPHdr *new_hdr, bool retain_proxy_auth_hdrs, ink_time_t date = 0);

static void convert_request(HTTPVersion outgoing_ver, HTTPHdr *outgoing_request);
static void convert_response(HTTPVersion outgoing_ver, HTTPHdr *outgoing_response);
static void convert_response(HTTPVersion outgoing_ver, HTTPHdr *outgoing_response, char const *reason_phrase = nullptr);
static void convert_to_1_0_request_header(HTTPHdr *outgoing_request);
static void convert_to_1_1_request_header(HTTPHdr *outgoing_request);
static void convert_to_1_0_response_header(HTTPHdr *outgoing_response);
static void convert_to_1_1_response_header(HTTPHdr *outgoing_response);
static void convert_to_1_0_response_header(HTTPHdr *outgoing_response, char const *reason_phrase = nullptr);
static void convert_to_1_1_response_header(HTTPHdr *outgoing_response, char const *reason_phrase = nullptr);

static ink_time_t calculate_document_age(ink_time_t request_time, ink_time_t response_time, HTTPHdr *base_response,
ink_time_t base_response_date, ink_time_t now);
Expand Down
15 changes: 11 additions & 4 deletions tests/gold_tests/h2/replay_h2origin/h1-client-h2-origin.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ sessions:
size: 0

proxy-response:
version: '2'
version: '1.1'
status: 200
reason: OK
headers:
encoding: esc_json
fields:
Expand Down Expand Up @@ -142,6 +143,7 @@ sessions:
proxy-response:
version: '1.1'
status: 200
reason: OK
headers:
encoding: esc_json
fields:
Expand Down Expand Up @@ -220,8 +222,9 @@ sessions:
size: 0

proxy-response:
version: '2'
version: '1.1'
status: 404
reason: Not Found
headers:
encoding: esc_json
fields:
Expand Down Expand Up @@ -286,8 +289,9 @@ sessions:
size: 32

proxy-response:
version: '2'
version: '1.1'
status: 200
reason: OK
headers:
encoding: esc_json
fields:
Expand Down Expand Up @@ -353,8 +357,9 @@ sessions:
size: 1600

proxy-response:
version: '2'
version: '1.1'
status: 200
reason: OK
headers:
encoding: esc_json
fields:
Expand Down Expand Up @@ -422,6 +427,7 @@ sessions:
proxy-response:
version: '1.1'
status: 200
reason: OK
headers:
encoding: esc_json
fields:
Expand Down Expand Up @@ -489,6 +495,7 @@ sessions:
proxy-response:
version: '1.1'
status: 200
reason: OK
headers:
encoding: esc_json
fields:
Expand Down