Skip to content
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.

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

HTTP Status Codes missing some IANA/industry-standard values #23458

Open
aetracht opened this issue Oct 12, 2022 · 3 comments
Open

HTTP Status Codes missing some IANA/industry-standard values #23458

aetracht opened this issue Oct 12, 2022 · 3 comments

Comments

@aetracht
Copy link

aetracht commented Oct 12, 2022

Title: The HTTP status code definitions are missing some well-know/standard values

Description:

Testing the request_handle:respond Lua function with various status codes, I find that the status text for the following codes is set to "Unknown" instead of the correct value:

  • 102: "Processing", # see RFC 2518
  • 103: "Early Hints", # see RFC 8297
  • 306: "Switch Proxy", # Switch Proxy (now unused)
  • 418: "I'm a teapot", # see RFC 2324 (now unused)
  • 425: "Too Early", # see RFC 8470
  • 449: "Retry With", # proprietary MS extension
  • 451: "Unavailable For Legal Reasons"

With the other status codes, like 200 or 301, the correct status text is return in the response ("OK" or "Moved Permanently", respectively). With the status codes listed above, the status text is "Unknown" instead of the IANA/industry values.

Searching the source code, it appears that these codes are just missing from the various HTTP status definitions.

All of these, with the possible exceptions of deprecated/non-standard 306, 418, and 449, should be added to the various lists of HTTP status codes in the Envoy source.

Sources:

Source code locations

10:24:18 envoy$ grep -ril "moved.*permanently" * 
api/envoy/config/route/v3/route_components.proto
api/envoy/type/v3/http_status.proto
api/envoy/type/http_status.proto
api/envoy/api/v2/route/route_components.proto
bazel/external/http_parser/http_parser.h
envoy/http/codes.h
source/common/http/codes.cc
source/common/router/config_utility.cc
source/common/router/config_impl.h
test/extensions/filters/http/ext_proc/ext_proc_grpc_fuzz_helper.cc
test/common/http/codes_test.cc
test/common/router/config_impl_test.cc
test/common/router/router_test.cc
10:24:26 envoy$ 

vs.

10:24:26 envoy$ grep -ril "unavailable.*for.*legal.*reasons" * 
bazel/external/http_parser/http_parser.h
10:25:00 envoy$ 

Repro steps:

Try the Lua code:

                              request_handle:respond(
                                {[":status"] = status_code}, "")

with the known status codes. Aside from 500-504, which appear to be caught as other errors, all of the Werkzeug/IANA values provide the correct status text except for those lists above, which say "Unknown"

Or just grep as shown above to check for the texts of these codes.

As a potential work-around, I tried using a string of "status_code status_text" in request_handle:response but that doesn't work and, instead, responds with 404 Not Found.

@aetracht aetracht added bug triage Issue requires triage labels Oct 12, 2022
@wbpcode wbpcode added help wanted Needs help! area/http and removed triage Issue requires triage labels Oct 17, 2022
@daixiang0
Copy link
Member

Actually some codes like 451 is defined as below:

XX(451, UNAVAILABLE_FOR_LEGAL_REASONS, Unavailable For Legal Reasons) \

@aetracht
Copy link
Author

@daixiang0 Indeed, my grep did find that one instance (see the end of my write-up), but that is insufficient for general use of all of the IANA/Industry-standard values.

@daixiang0
Copy link
Member

I think it is good to add those, you can post a PR to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants