Skip to content

Comments

Fix for missing content-length header when using QUIC#1501

Merged
mholt merged 6 commits intocaddyserver:masterfrom
ssut:fix_fastcgi_using_quic
Mar 10, 2017
Merged

Fix for missing content-length header when using QUIC#1501
mholt merged 6 commits intocaddyserver:masterfrom
ssut:fix_fastcgi_using_quic

Conversation

@ssut
Copy link

@ssut ssut commented Mar 7, 2017

If request.ContentLength is set then it will be used instead of getting it from request.Header map since quic-go(quic-go/quic-go@bb24be8) will not store (and pass) the Content-Length header using its header map.

ssut added 2 commits March 7, 2017 22:51
If request.ContentLength is set then it will be used instead of getting
it from request.Header map since quic-go(quic-go/quic-go@bb24be8)
will not store (and pass) the Content-Length header using its header
map.

This fixes a potential issue where FastCGI POST requests body empty when
QUIC is enabled. (caddyserver#1370)
quic-go uses int64 for contentLength
@ssut ssut changed the title Fix fastcgi using quic Fix for missing content-length header when using QUIC Mar 7, 2017
ssut and others added 4 commits March 7, 2017 23:18
the data type for contentLength
…ix_fastcgi_using_quic

* 'fix_fastcgi_using_quic' of github.com:ssut/caddy:
  Avoid panic if reloading before server is started
@mholt
Copy link
Member

mholt commented Mar 10, 2017

Thanks @ssut - did you verify that this fixes an issue? Is the issue documented?

@ssut
Copy link
Author

ssut commented Mar 10, 2017

@mholt I'm pretty sure this fixes the exact issue such like #1370, in this case #1483 (comment).

@mholt
Copy link
Member

mholt commented Mar 10, 2017

@ssut By "pretty sure" do you mean it doesn't work before your change, but does work after? If you're sure that's the case, then I'm okay with merging this.

@mholt mholt merged commit c62b6b9 into caddyserver:master Mar 10, 2017
@mholt
Copy link
Member

mholt commented Mar 10, 2017

Great, thank you @ssut!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants