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.

Sign up for GitHub

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

Jump to bottom

Allow the actual use of maximum method name length #12534

Closed
wants to merge 1 commit into from
Closed

Allow the actual use of maximum method name length #12534

wants to merge 1 commit into from

Conversation

miyagawa
Copy link
Contributor

@miyagawa miyagawa commented Dec 16, 2023 •
edited

While reviewing #12311 ("Increase the maximum request method name length from 11 to 23"), I tested a few requests and noticed that, before the change (curl 8.4.0), 11 character request method (which our internal tooling uses) was actually not allowed, while 10 characters did work. After the change (curl 8.5.0), request method with 23 characters long is still not allowed, while 22 characters long is allowed.

➜  curl --version                                                      
curl 8.5.0 (aarch64-apple-darwin23.0.0) libcurl/8.5.0 (SecureTransport) OpenSSL/3.2.0 zlib/1.2.12 brotli/1.1.0 zstd/1.5.5 libidn2/2.3.4 libssh2/1.11.0 nghttp2/1.58.0 librtmp/2.3 OpenLDAP/2.6.6

# 22 characters
➜  curl $URL -X GETGETGETGETGETGETGETG
<html>
<head><title>405 Not Allowed</title></head>
<body bgcolor="white">
<center><h1>405 Not Allowed</h1></center>
<hr><center>nginx/1.6.3</center>
</body>
</html>

# 23 characters
➜  curl $URL -X GETGETGETGETGETGETGETGE
curl: (43) Failed sending HTTP request

Looking at the code, it looks like there's an off-by-one error in the length check, disallowing the full use of 23 bytes (+ NUL).

@github-actions github-actions bot added the HTTP label Dec 16, 2023
@miyagawa miyagawa changed the title Allow the actual use of req->method array max length Allow the actual use maximum method name length Dec 16, 2023
@miyagawa miyagawa changed the title Allow the actual use maximum method name length Allow the actual use of maximum method name length Dec 16, 2023
@bagder
Copy link
Member

bagder commented Dec 16, 2023

Thanks!

@bagder bagder closed this in 1e9db69 Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bagder bagder bagder approved these changes

Assignees
No one assigned
Labels
HTTP
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@miyagawa @bagder