noahbackus/godot
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Make HTTPRequest 301 and 302 Redirects Standards-Compliant

Browse Source 
The behavior of 301 and 302 redirects in the HTTPRequest node are not
standards-compliant. Specifically, requests using unsafe methods were not
being changed to GET and their headers were not being modified. This
means that we were automatically redirecting POST, PUT, etc. requests
with empty bodies and the same headers. This can pose a security risk if
the server expects 301/302 responses to get changed to GET or if the
user doesn't expect unsafe methods to be automatically redirected.

Per
[RFC9110](https://www.rfc-editor.org/rfc/rfc9110#name-redirection-3xx),
the correct behavior is to change the method to GET for 301 and 302
redirections and remove any content headers as well as those related to
security contexts like "Authorization: ".

I have made these changes, so now the 301 and 302 redirects should
change any unsafe methods to GET and remove any sensitive headers.

GET, HEAD, OPTIONS, and TRACE requests that receive a 301 or 302 are
automatically forwarded unchanged since those methods are safe.

Co-authored-by: Fabio Alessandrelli <fabio.alessandrelli@gmail.com>
This commit is contained in:
Dalton Lang
2024-04-25 22:19:57 -05:00
committed by Rémi Verschelde
parent 4089843d13
commit 9cb9c28c3c
2 changed files with 34 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
scene/main/http_request.cpp
View File

@@ -209,6 +209,26 @@ void HTTPRequest::cancel_request() {
requesting = false;
}
bool HTTPRequest::_is_content_header(const String &p_header) const {
return (p_header.begins_with("content-type:") || p_header.begins_with("content-length:") || p_header.begins_with("content-location:") || p_header.begins_with("content-encoding:") ||
p_header.begins_with("transfer-encoding:") || p_header.begins_with("connection:") || p_header.begins_with("authorization:"));
}
bool HTTPRequest::_is_method_safe() const {
return (method == HTTPClient::METHOD_GET || method == HTTPClient::METHOD_HEAD || method == HTTPClient::METHOD_OPTIONS || method == HTTPClient::METHOD_TRACE);
}
Error HTTPRequest::_get_redirect_headers(Vector<String> *r_headers) {
for (const String &E : headers) {
const String h = E.to_lower();
// We strip content headers when changing a redirect to GET.
if (!_is_content_header(h)) {
r_headers->push_back(E);
}
}
return OK;
}
bool HTTPRequest::_handle_response(bool *ret_value) {
if (!client->has_response()) {
_defer_done(RESULT_NO_RESPONSE, 0, PackedStringArray(), PackedByteArray());
@@ -268,6 +288,16 @@ bool HTTPRequest::_handle_response(bool *ret_value) {
final_body_size.set(0);
redirections = new_redirs;
*ret_value = false;
if (!_is_method_safe()) {
// 301, 302, and 303 are changed to GET for unsafe methods.
// See: https://www.rfc-editor.org/rfc/rfc9110#section-15.4-3.1
method = HTTPClient::METHOD_GET;
// Content headers should be dropped if changing method.
// See: https://www.rfc-editor.org/rfc/rfc9110#section-15.4-6.2.1
Vector<String> req_headers;
_get_redirect_headers(&req_headers);
headers = req_headers;
}
return true;
}
}

4
scene/main/http_request.h
View File

@@ -102,6 +102,10 @@ private:
void _redirect_request(const String &p_new_url);
bool _is_content_header(const String &p_header) const;
bool _is_method_safe() const;
Error _get_redirect_headers(Vector<String> *r_headers);
bool _handle_response(bool *ret_value);
Error _parse_url(const String &p_url);