search

sablierapp / sablier

Start your containers on demand, shut them down automatically when there's no activity. Docker, Docker Swarm Mode and Kubernetes compatible.
https://sablierapp.dev/
GNU Affero General Public License v3.0
1.36k stars 46 forks source link

Fix issue with loading page not showing in Traefik for h2 connections #173

Closed patcher-ms closed 1 year ago

patcher-ms commented 1 year ago

I continued my investigation of issue https://github.com/acouvreur/sablier/issues/165 and I discovered that when making a working request these are the headers seen by cURL:

curl -v http://localhost:8080/dynamic/whoami
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /dynamic/whoami HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.88.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< Content-Type: text/html
< Date: Sun, 27 Aug 2023 00:23:52 GMT
< Transfer-Encoding: chunked
<

Note that Content-Length is not present. https://stackoverflow.com/questions/75091383/why-does-golang-http-responsewriter-auto-add-content-length-if-its-no-more-than talks about this and describes what's going on.

Apparently this is not an issue for HTTP/1.1 connections, which are used by cURL and the browsers when there is no TLS involved. When the request is HTTPs cURL and browsers prefer to use h2, which I guess handle headers in a different ways.

If I force cURL to use HTTP1.1 I get the loading page even for TLS requests, so this is definitely and issue related to h2. Anyway, Content-Length is never explicitly set in serveDynamic and loading pages tend to be above the threshold so I think not propagating it is the right fix for the issue.

patcher-ms commented 1 year ago

@acouvreur do you have any thoughts on this?

Sorry for the ping, earlier you replied to my other issue and I wanted to make sure this PR didn’t fall through the cracks :)

acouvreur commented 1 year ago

Will check if any side effect might occur without sending the content length

But I might merge it soon :)

acouvreur commented 1 year ago

:tada: This PR is included in version 1.4.0-beta.7 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

acouvreur commented 1 year ago

:tada: This PR is included in version 1.4.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

acouvreur commented 1 year ago

@all-contributors please add @patcher-ms for bug and code

allcontributors[bot] commented 1 year ago

@acouvreur

@patcher-ms already contributed before to bug, code