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

[mbertschler]

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?

go/src/net/http/fs.go

Line 284 in a610957

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]

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]

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.

[mholt]

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