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

net/http: ServeContent doesn't set Content-Length when Content-Encoding is set #19420

Closed
mbertschler opened this issue Mar 6, 2017 · 3 comments

Comments

@mbertschler
Copy link

What version of Go are you using (go version)?

go version go1.8 darwin/amd64

What did you do?

A HTTP request served by caddy for /myfile. If the request is sent with accept encoding gzip, caddy has a mechanism that looks up if a file /myfile.gz is present in the folder, and serves that directly instead of gzipping the uncompressed myfile.

Relevant lines are starting here:
https://github.com/mbertschler/caddy/blob/2649f0330e3d4ab3395618eeb2c6705a5d240efa/caddyhttp/staticfiles/fileserver.go#L203

What did you expect to see?

A HTTP response that has a Content-Length header set.

What did you see instead?

The HTTP response doesn't have a Content-Length header set.

This happens because the serveContent function only sets Content-Length if Content-Encoding is empty. In caddy's case it is set to gzip before ServeContent is called. This code was written 5 years ago by @bradfitz when ServeContent was first added. Do you still know why this check was added?

if w.Header().Get("Content-Encoding") == "" {

What operating system and processor architecture are you using (go env)?

$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/mbertschler/go:/Users/mbertschler/wcd/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.8/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.8/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/d0/zv0zzgf90r9bds00x6rm_qbr0000gn/T/go-build936143375=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
@kevinburke
Copy link
Contributor

The Content-Length is supposed to be set to the compressed size, per http://stackoverflow.com/a/3819303/329700, I believe. It's probably better to omit it than to send an incorrect value.

I think the problem in this case is that the sendSize represents the uncompressed size.

@bradfitz
Copy link
Contributor

bradfitz commented Mar 6, 2017

It's never necessary to set Content-Length. If Caddy wants to set its own headers before ServeContent, then it can also set Content-Length. ServeContent is being conservative here and not interfering if the caller is trying to do its own thing.

I'm going to close this for now, but feel free to file a bug again Caddy, or @mholt can argue why ServeContent is doing the wrong thing.

@bradfitz bradfitz closed this as completed Mar 6, 2017
@mholt
Copy link

mholt commented Mar 6, 2017

Ah, just looked at the spec again. You're both right, I didn't realize Content-Length was optional. ServeContent is fine. Thanks!

@golang golang locked and limited conversation to collaborators Mar 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants