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

Web export HTTPRequest returns RESULT_CANT_CONNECT when API uses compression #79327

Closed
SanteriLoitomaa opened this issue Jul 11, 2023 · 5 comments · Fixed by #79846
Closed

Web export HTTPRequest returns RESULT_CANT_CONNECT when API uses compression #79327

SanteriLoitomaa opened this issue Jul 11, 2023 · 5 comments · Fixed by #79846

Comments

@SanteriLoitomaa
Copy link

SanteriLoitomaa commented Jul 11, 2023

Godot version

v4.1.stable.official [9704596]
EDIT: Also v4.1.1.stable.official [bd6af8e]

System information

Godot v4.1.stable - Windows 10.0.19045 - Vulkan (Compatibility) - NVIDIA GeForce GTX 1060 6GB (NVIDIA; 31.0.15.3640) - Intel(R) Core(TM) i5-7400 CPU @ 3.00GHz (4 Threads)

Issue description

Using HTTPRequest request() on a Web export with accept_gzip = false on an API that can use compression results in a RESULT_CANT_CONNECT error [2, 0, [], []]. This does not happen on any other export nor can I find a reason for it to fail as the API in question can also return uncompressed data if the Accept-Encoding header is not used.

Steps to reproduce

var http:HTTPRequest = HTTPRequest.new()
add_child(http)
http.accept_gzip = false
http.request("https://dev.playkantti.fi/ok.php")
var result = await http.request_completed
print(result)

The result of this code will be normal on every export except on Web where it will be [2, 0, [], []]. You can also use the project and web export below to try it out. It's a bit more visual than the code above.

Minimal reproduction project

Web export: https://dev.playkantti.fi/
Project: NetworkTest.zip

@bsarsgard
Copy link

I'm also seeing this issue, as well as a potentially related issue (that was present in 4.0 as well, whereas this issue is new in 4.1) where an HttpRequest with "accept_gzip = true" (or not set at all since this is the default) returns response code 8 "RESULT_BODY_DECOMPRESS_FAILED".

@Calinou
Copy link
Member

Calinou commented Jul 20, 2023

cc @Faless

Can you reproduce this when performing a HTTP request in a native export (i.e. not a web export)?

@bsarsgard
Copy link

bsarsgard commented Jul 20, 2023

Everything works as expected in a windows export. One thing I'm noticing is that when I examine the network trace in the html5 export, it's sending a request header "Accept-Encoding: gzip, deflate, br" when I have accept_gzip set to false and even explicitly send my own "Accept-Encoding: identity" in the headers. I may be missing something here though, as going through the cpp code for HttpRequest I don't see anywhere that could be replacing that header.

edit: Actually disregard the bit about accept-encoding, I don't think that's it because that behavior is identical in 4.0 but it works just fine. If you open the above example in both 4.0 and 4.1, you can see that while both work fine locally or in a windows export, exporting to html5 works with 4.0 but is broken in 4.1. If you look at the network trace, everything appears 100% identical between versions as far as I can tell, so it must be some failure in handling the response.

@Faless
Copy link
Collaborator

Faless commented Jul 20, 2023

This is a regression from #77648 , see #47597 for the rationale.
We can't use Content-Length on the web, as that is not the size that you will read when the body is compressed (browsers automatically perform decompression).
We also can't check for the Content-Encoding as that's a "protected header", see Access-Control-Expose-Headers and CORS-safelisted_response_header.
I did mention these topics to the W3C, but their basically said it's not going to change, and that actually more might be removed.

I'll open a PR to revert that change and better comment so we don't regress again.

@Faless
Copy link
Collaborator

Faless commented Jul 20, 2023

even explicitly send my own "Accept-Encoding: identity" in the headers.

For similar reasons as above, we can't specify those headers ( see Forbidden headers ), browsers simply do not want applications to control them, so there isn't much we can do about it.

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.

6 participants