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

response code 418 results in 500 Internal Server Error #144

Closed
akostadinov opened this issue Jul 6, 2023 · 1 comment · Fixed by #145
Closed

response code 418 results in 500 Internal Server Error #144

akostadinov opened this issue Jul 6, 2023 · 1 comment · Fixed by #145

Comments

@akostadinov
Copy link
Contributor

System Information

  • OS: Fedora 38
  • Ruby: 3.2.2
  • Version: 0.7.55
  • OpenSSL: OpenSSL 3.0.9 30 May 2023

Description

I've hit a serious issue. When trying to reply with 418 I'm a teapot response what is being sent is actually 500 Internal Server Error. Users may be severely confused over the failure cause.

Rack App to Reproduce

            app = proc do |env|
                body = "Upgrade connection to Websocket to enter echo mode."
                [418, { "Content-Length" => body.bytesize, "Content-Type" => "text/plain" }, [body]]
            end

            Iodine.listen service: :ws, address: "::", port: 3003, handler: app
            Iodine.start

Testing code

curl http://localhost:3003/

Expected behavior

< HTTP/1.1 418 I'm a teapot
< connection:keep-alive
< content-length:51
< content-type:text/plain
< date:Thu, 06 Jul 2023 12:25:24 GMT

Actual behavior

< HTTP/1.1 500 Internal Server Error
< connection:keep-alive
< content-length:51
< content-type:text/plain
< date:Thu, 06 Jul 2023 12:26:48 GMT
@boazsegev
Copy link
Owner

Yes, true, the April Fool's Day RFC isn't implemented...

You're welcome to add support / PR:

iodine/ext/iodine/http.c

Lines 2698 to 2699 in e5560c5

HTTP_SET_STATUS_STR(417, "Expectation Failed"),
HTTP_SET_STATUS_STR(421, "Misdirected Request"),

Cheers,
B.

FYI: I didn't plan on implementing this in the facil.io 0.8.x version, but if you know of a reason why it's a good thing to add, I'll add support.

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

Successfully merging a pull request may close this issue.

2 participants