You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bneradt (via GitHub)" <gi...@apache.org> on 2023/04/15 17:59:45 UTC
[GitHub] [trafficserver] bneradt opened a new pull request, #9615: Ensure a reason phrase when sending an HTTP/1 response
bneradt opened a new pull request, #9615:
URL: https://github.com/apache/trafficserver/pull/9615
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a diff in pull request #9615: Ensure a reason phrase when sending an HTTP/1 response
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9615:
URL: https://github.com/apache/trafficserver/pull/9615#discussion_r1170295962
##########
proxy/http/HttpTransact.cc:
##########
@@ -7834,7 +7834,13 @@ 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);
+ } else if (base_response->status_get() != HTTP_STATUS_NONE) {
Review Comment:
I agree. We're best off making sure it isn't nullptr to be safe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a diff in pull request #9615: Ensure a reason phrase when sending an HTTP/1 response
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9615:
URL: https://github.com/apache/trafficserver/pull/9615#discussion_r1169111595
##########
proxy/http/HttpTransact.cc:
##########
@@ -7834,7 +7834,13 @@ 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) {
+ Debug("http_transact", "Using reason phrase from status %d: %s", status_code, reason_phrase);
Review Comment:
reason_phrase is always nullptr here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt merged pull request #9615: Ensure a reason phrase when sending an HTTP/1 response
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #9615:
URL: https://github.com/apache/trafficserver/pull/9615
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a diff in pull request #9615: Ensure a reason phrase when sending an HTTP/1 response
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9615:
URL: https://github.com/apache/trafficserver/pull/9615#discussion_r1169116452
##########
proxy/http/HttpTransact.cc:
##########
@@ -7834,7 +7834,13 @@ 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) {
+ Debug("http_transact", "Using reason phrase from status %d: %s", status_code, reason_phrase);
Review Comment:
Good catch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a diff in pull request #9615: Ensure a reason phrase when sending an HTTP/1 response
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9615:
URL: https://github.com/apache/trafficserver/pull/9615#discussion_r1169374116
##########
proxy/http/HttpTransact.cc:
##########
@@ -7834,7 +7834,13 @@ 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);
+ } else if (base_response->status_get() != HTTP_STATUS_NONE) {
Review Comment:
We may want to check the availability of `base_response`, although it doesn't make sense if it has `nullptr` on this line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org