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 2024/04/19 18:31:29 UTC

[PR] HTTP 418: "I'm a teapot" reason string [trafficserver]

bneradt opened a new pull request, #11273:
URL: https://github.com/apache/trafficserver/pull/11273

   Add reason string "I'm a teapot" support for HTTP response status 418. ATS core will almost certainly not use this, but plugins may for testing purposes. It's handy for those circumstances to add the response reason support for the 418 status.
   
   ---
   # For Review
   
   Unfortunately, c macro [stringification](https://gcc.gnu.org/onlinedocs/gcc-4.1.2/cpp/Stringification.html) does not support the ``'`` character in strings. So the vast majority of this review is simply converting away from the reliance upon stringification to, instead, explicitly quoted reason phrases. The only new line added is the `418` status containing `I'm a teapot`.


-- 
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


Re: [PR] HTTP 418: "I'm a teapot" reason string [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #11273:
URL: https://github.com/apache/trafficserver/pull/11273#issuecomment-2069892350

   > I'd say relying on reason strings is a bad custom even for testing because such a test is not runnable on H2 and H3. I don't think we should open wide the door for making tests that won't work for H2/3.
   
   Agreed. My separate change made the plugin code handle an absent reason string:
   https://github.com/apache/trafficserver/pull/11272
   
   That said, is there a problem having a reason string for 418?


-- 
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


Re: [PR] HTTP 418: "I'm a teapot" reason string [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #11273:
URL: https://github.com/apache/trafficserver/pull/11273#issuecomment-2067130231

   [approve ci autest]


-- 
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


Re: [PR] HTTP 418: "I'm a teapot" reason string [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt closed pull request #11273: HTTP 418: "I'm a teapot" reason string
URL: https://github.com/apache/trafficserver/pull/11273


-- 
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


Re: [PR] HTTP 418: "I'm a teapot" reason string [trafficserver]

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #11273:
URL: https://github.com/apache/trafficserver/pull/11273#issuecomment-2069851244

   I'd say relying on reason strings is a bad custom even for testing because such a test is not runnable on H2 and H3. I don't think we should open wide the door for making tests that won't work for H2/3.


-- 
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


Re: [PR] HTTP 418: "I'm a teapot" reason string [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #11273:
URL: https://github.com/apache/trafficserver/pull/11273#issuecomment-2067162881

   [approve ci autest]


-- 
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


Re: [PR] HTTP 418: "I'm a teapot" reason string [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #11273:
URL: https://github.com/apache/trafficserver/pull/11273#issuecomment-2068261903

   There were discussions about `418 I'm a teapot` at httpbis. Now, RFC 9110 says `418` is "unused" as HTTP Status Code.
   
   https://www.rfc-editor.org/rfc/rfc9110.html#section-15.5.19
   https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml
   
   Personally, I love the humor of `Hyper Text Coffee Pot Control Protocol`, but we don't need to add the reason string to ATS as of 2024, because it's not the `Hyper Text Transfer Protocol`.


-- 
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


Re: [PR] HTTP 418: "I'm a teapot" reason string [trafficserver]

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #11273:
URL: https://github.com/apache/trafficserver/pull/11273#issuecomment-2069939467

   I guess there's no problem on runtime (assuming the change on this PR has no impact to performance), but what's the motivation for having the reason string if we agree on that we shouldn't use it? We'd have to check whether future PRs don't use the reason string.


-- 
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