-
Notifications
You must be signed in to change notification settings - Fork 30
allow empty header values #139
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
Conversation
| template <class T> size_t name_max(const std::unordered_map<std::string, T>& methods) { | ||
| template <class T> | ||
| size_t name_max(const std::unordered_map<std::string, T>& methods) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stupid linters
| if ((field_end = partial_buffer.find(':')) == std::string::npos) | ||
| throw RESPONSE_400; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so we still bail if you send headers with no : to separate name and value
| if ((value_begin = partial_buffer.find_first_not_of(' ', field_end + 1)) == | ||
| std::string::npos) | ||
| value_begin = partial_buffer.size(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but now if you dont send a value or if your value is all whitespace we detect that and set the value to empty string
| testable_http_server_t server(context, "ipc:///tmp/test_http_server", | ||
| "ipc:///tmp/test_http_proxy_upstream", "ipc:///tmp/test_http_results", | ||
| "ipc:///tmp/test_http_interrupt", false, | ||
| MAX_REQUEST_SIZE, -1, | ||
| testable_http_server_t server( | ||
| context, "ipc:///tmp/test_http_server", "ipc:///tmp/test_http_proxy_upstream", | ||
| "ipc:///tmp/test_http_results", "ipc:///tmp/test_http_interrupt", false, MAX_REQUEST_SIZE, -1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more linter blowback
| request_str = "GET /empty HTTP/1.1\r\nx-often-empty-header: \r\n\r\n"; | ||
| request = http_request_t::from_string(request_str.c_str(), request_str.size()); | ||
| auto empty = request.headers.find("x-often-empty-header"); | ||
| if (empty == request.headers.cend() || !empty->second.empty()) | ||
| throw std::runtime_error("Request parsing failed"); | ||
|
|
||
| request_str = "GET /empty HTTP/1.1\r\nx-often-empty-header:\r\n\r\n"; | ||
| request = http_request_t::from_string(request_str.c_str(), request_str.size()); | ||
| auto still_empty = request.headers.find("x-often-empty-header"); | ||
| if (still_empty == request.headers.cend() || !still_empty->second.empty()) | ||
| throw std::runtime_error("Request parsing failed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we try with no value but a ton of whitespace and then again with literally nothing, then below we make sure that no colon still fails
@nilsnolde is making good progress on vector tile serving and noticed an issue when hooking it up to qgis. prime_server chokes on the requests. he did all the hard work of debugging it to the section of code that parses header names and values. basic gist is, prime_server wasnt allowing empty values. i guess i didnt read the rfc all that carefully.. anyway this pr adds tests for and fixes that.