Skip to content

Commit 487577c

Browse files
bneradtbneradt
committed
Ensure a reason phrase when sending an HTTP/1 response
With an HTTP/1 client and an HTTP/2 origin, responses will come from the origin without reason phrases because reason phrases are explicitly removed from the HTTP/2 RFC. When expanding test coverage of reason phrases it was noticed that when converting responses from HTTP/2 to HTTP/1 to send toward the client, the reason phrases were empty. This updates the conversion logic to ensure that HTTP/1 clients receive a reason phrase.
1 parent 6065340 commit 487577c

File tree

4 files changed

+48
-14
lines changed

4 files changed

+48
-14
lines changed

‎proxy/http/HttpTransact.cc‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7834,7 +7834,13 @@ HttpTransact::build_response(State *s, HTTPHdr *base_response, HTTPHdr *outgoing
78347834
HTTPStatus status_code, const char *reason_phrase)
78357835
{
78367836
if (reason_phrase == nullptr) {
7837-
reason_phrase = http_hdr_reason_lookup(status_code);
7837+
if (status_code != HTTP_STATUS_NONE) {
7838+
reason_phrase = http_hdr_reason_lookup(status_code);
7839+
Debug("http_transact", "Using reason phrase from status %d: %s", status_code, reason_phrase);
7840+
} else if (base_response->status_get() != HTTP_STATUS_NONE) {
7841+
reason_phrase = http_hdr_reason_lookup(base_response->status_get());
7842+
Debug("http_transact", "Using reason phrase from base_response status %d: %s", status_code, reason_phrase);
7843+
}
78387844
}
78397845

78407846
if (base_response == nullptr) {
@@ -7942,7 +7948,15 @@ HttpTransact::build_response(State *s, HTTPHdr *base_response, HTTPHdr *outgoing
79427948
HttpTransactHeaders::insert_via_header_in_response(s, outgoing_response);
79437949
}
79447950

7945-
HttpTransactHeaders::convert_response(outgoing_version, outgoing_response);
7951+
// When converting a response, only set a reason phrase if one was not already
7952+
// set via some explicit call above.
7953+
char const *reason_phrase_for_convert = nullptr;
7954+
int outgoing_reason_phrase_len = 0;
7955+
char const *outgoing_reason_phrase = outgoing_response->reason_get(&outgoing_reason_phrase_len);
7956+
if (outgoing_reason_phrase == nullptr || outgoing_reason_phrase_len == 0) {
7957+
reason_phrase_for_convert = reason_phrase;
7958+
}
7959+
HttpTransactHeaders::convert_response(outgoing_version, outgoing_response, reason_phrase_for_convert);
79467960

79477961
// process reverse mappings on the location header
79487962
// TS-1364: do this regardless of response code

‎proxy/http/HttpTransactHeaders.cc‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -269,12 +269,12 @@ HttpTransactHeaders::convert_request(HTTPVersion outgoing_ver, HTTPHdr *outgoing
269269
////////////////////////////////////////////////////////////////////////
270270
// Just convert the outgoing response to the appropriate version
271271
void
272-
HttpTransactHeaders::convert_response(HTTPVersion outgoing_ver, HTTPHdr *outgoing_response)
272+
HttpTransactHeaders::convert_response(HTTPVersion outgoing_ver, HTTPHdr *outgoing_response, char const *reason_phrase)
273273
{
274274
if (outgoing_ver == HTTPVersion(1, 1)) {
275-
convert_to_1_1_response_header(outgoing_response);
275+
convert_to_1_1_response_header(outgoing_response, reason_phrase);
276276
} else if (outgoing_ver == HTTPVersion(1, 0)) {
277-
convert_to_1_0_response_header(outgoing_response);
277+
convert_to_1_0_response_header(outgoing_response, reason_phrase);
278278
} else {
279279
Debug("http_trans", "[HttpTransactHeaders::convert_response]"
280280
"Unsupported Version - passing through");
@@ -325,7 +325,7 @@ HttpTransactHeaders::convert_to_1_1_request_header(HTTPHdr *outgoing_request)
325325
////////////////////////////////////////////////////////////////////////
326326
// Take an existing outgoing response header and make it HTTP/1.0
327327
void
328-
HttpTransactHeaders::convert_to_1_0_response_header(HTTPHdr *outgoing_response)
328+
HttpTransactHeaders::convert_to_1_0_response_header(HTTPHdr *outgoing_response, char const *reason_phrase)
329329
{
330330
// // These are required
331331
// ink_assert(outgoing_response->status_get());
@@ -334,6 +334,12 @@ HttpTransactHeaders::convert_to_1_0_response_header(HTTPHdr *outgoing_response)
334334
// Set HTTP version to 1.0
335335
outgoing_response->version_set(HTTPVersion(1, 0));
336336

337+
// Set reason phrase if passed in.
338+
if (reason_phrase != nullptr) {
339+
Debug("http_transact_headers", "Setting HTTP/1.0 reason phrase to '%s'", reason_phrase);
340+
outgoing_response->reason_set(reason_phrase, strlen(reason_phrase));
341+
}
342+
337343
// Keep-Alive?
338344

339345
// Cache-Control?
@@ -342,13 +348,20 @@ HttpTransactHeaders::convert_to_1_0_response_header(HTTPHdr *outgoing_response)
342348
////////////////////////////////////////////////////////////////////////
343349
// Take an existing outgoing response header and make it HTTP/1.1
344350
void
345-
HttpTransactHeaders::convert_to_1_1_response_header(HTTPHdr *outgoing_response)
351+
HttpTransactHeaders::convert_to_1_1_response_header(HTTPHdr *outgoing_response, char const *reason_phrase)
346352
{
353+
Debug("http_transact_headers", "Entering convert_to_1_1_response_header with reason phrase to '%s'", reason_phrase);
347354
// These are required
348355
ink_assert(outgoing_response->status_get());
349356

350357
// Set HTTP version to 1.1
351358
outgoing_response->version_set(HTTPVersion(1, 1));
359+
360+
// Set reason phrase if passed in.
361+
if (reason_phrase != nullptr) {
362+
Debug("http_transact_headers", "Setting HTTP/1.1 reason phrase to '%s'", reason_phrase);
363+
outgoing_response->reason_set(reason_phrase, strlen(reason_phrase));
364+
}
352365
}
353366

354367
///////////////////////////////////////////////////////////////////////////////

‎proxy/http/HttpTransactHeaders.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@ class HttpTransactHeaders
4444
static void copy_header_fields(HTTPHdr *src_hdr, HTTPHdr *new_hdr, bool retain_proxy_auth_hdrs, ink_time_t date = 0);
4545

4646
static void convert_request(HTTPVersion outgoing_ver, HTTPHdr *outgoing_request);
47-
static void convert_response(HTTPVersion outgoing_ver, HTTPHdr *outgoing_response);
47+
static void convert_response(HTTPVersion outgoing_ver, HTTPHdr *outgoing_response, char const *reason_phrase = nullptr);
4848
static void convert_to_1_0_request_header(HTTPHdr *outgoing_request);
4949
static void convert_to_1_1_request_header(HTTPHdr *outgoing_request);
50-
static void convert_to_1_0_response_header(HTTPHdr *outgoing_response);
51-
static void convert_to_1_1_response_header(HTTPHdr *outgoing_response);
50+
static void convert_to_1_0_response_header(HTTPHdr *outgoing_response, char const *reason_phrase = nullptr);
51+
static void convert_to_1_1_response_header(HTTPHdr *outgoing_response, char const *reason_phrase = nullptr);
5252

5353
static ink_time_t calculate_document_age(ink_time_t request_time, ink_time_t response_time, HTTPHdr *base_response,
5454
ink_time_t base_response_date, ink_time_t now);

‎tests/gold_tests/h2/replay_h2origin/h1-client-h2-origin.yaml‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,9 @@ sessions:
7272
size: 0
7373

7474
proxy-response:
75-
version: '2'
75+
version: '1.1'
7676
status: 200
77+
reason: OK
7778
headers:
7879
encoding: esc_json
7980
fields:
@@ -142,6 +143,7 @@ sessions:
142143
proxy-response:
143144
version: '1.1'
144145
status: 200
146+
reason: OK
145147
headers:
146148
encoding: esc_json
147149
fields:
@@ -220,8 +222,9 @@ sessions:
220222
size: 0
221223

222224
proxy-response:
223-
version: '2'
225+
version: '1.1'
224226
status: 404
227+
reason: Not Found
225228
headers:
226229
encoding: esc_json
227230
fields:
@@ -286,8 +289,9 @@ sessions:
286289
size: 32
287290

288291
proxy-response:
289-
version: '2'
292+
version: '1.1'
290293
status: 200
294+
reason: OK
291295
headers:
292296
encoding: esc_json
293297
fields:
@@ -353,8 +357,9 @@ sessions:
353357
size: 1600
354358

355359
proxy-response:
356-
version: '2'
360+
version: '1.1'
357361
status: 200
362+
reason: OK
358363
headers:
359364
encoding: esc_json
360365
fields:
@@ -422,6 +427,7 @@ sessions:
422427
proxy-response:
423428
version: '1.1'
424429
status: 200
430+
reason: OK
425431
headers:
426432
encoding: esc_json
427433
fields:
@@ -489,6 +495,7 @@ sessions:
489495
proxy-response:
490496
version: '1.1'
491497
status: 200
498+
reason: OK
492499
headers:
493500
encoding: esc_json
494501
fields:

0 commit comments

Comments
 (0)