Test enabling transport compression on ociserver

conformance/Dockerfile

@@ -1,6 +1,7 @@
 FROM golang:1.25-alpine AS build
 
-ARG DISTRIBUTION_SPEC_REF=967efdc079b91785ad18c77cc4f8991a47feefbf
+# PR https://github.com/opencontainers/distribution-spec/pull/624
+ARG DISTRIBUTION_SPEC_REF=316054f54c0192c4b3876b587d61f4bb931e7b0a
 
 RUN apk add --no-cache ca-certificates git
 RUN mkdir /src \

ociserver/blobs.go

@@ -58,61 +58,20 @@ func (s *Server) blobHeadGet() http.HandlerFunc {
 
 			var br io.ReadCloser
 			if rh != "" {
-				if !strings.HasPrefix(rh, "bytes=") {
-					returnError(w, ErrRangeNotSatisfiable("Range header must use bytes unit"))
+				bi, ei, parseErr := parseBlobRange(rh, blob.Size)
+				if parseErr != nil {
+					returnError(w, ErrRangeNotSatisfiable(parseErr.Error()))
 					return
 				}
-				rangeStart, rangeEnd, _ := strings.Cut(strings.TrimPrefix(rh, "bytes="), "-")
-				var bi, ei int64
-				if rangeStart == "" {
-					// Suffix range (e.g. bytes=-500): return the last N bytes.
-					suffixLen, parseErr := strconv.ParseInt(rangeEnd, 10, 64)
-					if parseErr != nil || suffixLen < 0 {
-						returnError(w, ErrRangeNotSatisfiable("invalid range"))
-						return
-					}
-					bi = blob.Size - suffixLen
-					if bi < 0 {
-						bi = 0
-					}
-					ei = blob.Size - 1
-				} else {
-					var parseErr error
-					bi, parseErr = strconv.ParseInt(rangeStart, 10, 64)
-					if parseErr != nil || bi < 0 {
-						returnError(w, ErrRangeNotSatisfiable("invalid range start"))
-						return
-					}
-					if rangeEnd == "" {
-						ei = blob.Size - 1
-					} else {
-						ei, parseErr = strconv.ParseInt(rangeEnd, 10, 64)
-						if parseErr != nil || ei < 0 {
-							returnError(w, ErrRangeNotSatisfiable("invalid range end"))
-							return
-						}
-					}
-				}
-				if ei >= blob.Size {
-					ei = blob.Size - 1
-				}
-				if bi >= blob.Size {
-					returnError(w, ErrRangeNotSatisfiable("range start is beyond end of blob"))
-					return
-				}
-				if ei < bi {
-					returnError(w, ErrRangeNotSatisfiable("range end is before range start"))
-					return
-				}
-				br, err = s.db.GetBlobRange(r.Context(), name, dgst, bi, ei+1)
+				br, err = s.db.GetBlobRange(r.Context(), name, dgst, bi, ei)
 				if err != nil {
 					s.logError(r.Context(), "getting blob range", err, "repository", name, "digest", dgstStr)
 					returnError(w, ErrServerError())
 					return
 				}
-				length := ei - bi + 1
+				length := ei - bi
 				w.Header().Set("Content-Length", fmt.Sprintf("%d", length))
-				w.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", bi, ei, blob.Size))
+				w.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", bi, ei-1, blob.Size))
 				w.Header().Set("Content-Type", "application/octet-stream")
 				w.Header().Set("Docker-Content-Digest", blob.Digest.String())
 				w.WriteHeader(http.StatusPartialContent)
@@ -144,6 +103,56 @@ func (s *Server) blobHeadGet() http.HandlerFunc {
 	}
 }
 
+// parseBlobRange parses a single HTTP byte range and returns its half-open
+// bounds within a blob of the given size.
+func parseBlobRange(rangeHeader string, blobSize int64) (int64, int64, error) {
+	const bytesPrefix = "bytes="
+	if !strings.HasPrefix(rangeHeader, bytesPrefix) {
+		return 0, 0, errors.New("range header must use bytes unit")
+	}
+
+	rangeStart, rangeEnd, ok := strings.Cut(strings.TrimPrefix(rangeHeader, bytesPrefix), "-")
+	if !ok {
+		return 0, 0, errors.New("invalid range")
+	}
+
+	var start, end int64
+	if rangeStart == "" {
+		// Suffix range (e.g. bytes=-500): return the last N bytes.
+		suffixLen, err := strconv.ParseInt(rangeEnd, 10, 64)
+		if err != nil || suffixLen < 0 {
+			return 0, 0, errors.New("invalid range")
+		}
+		start = max(blobSize-suffixLen, 0)
+		end = blobSize - 1
+	} else {
+		var err error
+		start, err = strconv.ParseInt(rangeStart, 10, 64)
+		if err != nil || start < 0 {
+			return 0, 0, errors.New("invalid range start")
+		}
+		if rangeEnd == "" {
+			end = blobSize - 1
+		} else {
+			end, err = strconv.ParseInt(rangeEnd, 10, 64)
+			if err != nil || end < 0 {
+				return 0, 0, errors.New("invalid range end")
+			}
+		}
+	}
+
+	if end >= blobSize {
+		end = blobSize - 1
+	}
+	if start >= blobSize {
+		return 0, 0, errors.New("range start is beyond end of blob")
+	}
+	if end < start {
+		return 0, 0, errors.New("range end is before range start")
+	}
+	return start, end + 1, nil
+}
+
 func (s *Server) blobDelete() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := mux.URLParam(r, "name")

ociserver/compression.go

@@ -0,0 +1,164 @@
+package ociserver
+
+import (
+	"bytes"
+	"compress/flate"
+	"compress/gzip"
+	"io"
+	"net/http"
+	"strings"
+)
+
+const acceptEncodingHeader = "Accept-Encoding"
+
+// compressedFormatMagic holds the leading bytes ("magic numbers") of file
+// formats that are already compressed. A response body starting with one of
+// these prefixes gains nothing -- and can even grow slightly -- from an
+// additional gzip/deflate pass, so compressHandler skips re-compressing it.
+//
+// Detection works on the blob's actual bytes, not any declared media type:
+// a blob referenced only by digest has no media type of its own (that's a
+// property of a manifest's reference to it, and this registry never even
+// records one more specific than "application/octet-stream" -- see
+// registry/storage/blobwriter.go), so media type isn't a usable signal here.
+// Sniffing real bytes instead also means this keeps working for any artifact
+// type: if the ecosystem ever stops shipping gzip-compressed tarballs by
+// convention and leans on transport compression instead, an uncompressed tar
+// simply won't match any of these prefixes and will still get gzip'd on the
+// wire like any other compressible content.
+var compressedFormatMagic = [][]byte{
+	{0x1f, 0x8b},                         // gzip
+	{0x28, 0xb5, 0x2f, 0xfd},             // zstd
+	{0x42, 0x5a, 0x68},                   // bzip2
+	{0xfd, 0x37, 0x7a, 0x58, 0x5a, 0x00}, // xz
+	{0x50, 0x4b, 0x03, 0x04},             // zip
+	{0x04, 0x22, 0x4d, 0x18},             // lz4
+}
+
+func looksAlreadyCompressed(head []byte) bool {
+	for _, magic := range compressedFormatMagic {
+		if bytes.HasPrefix(head, magic) {
+			return true
+		}
+	}
+	return false
+}
+
+// negotiateEncoding picks gzip or deflate from the client's Accept-Encoding
+// header. It intentionally doesn't parse quality values, matching the
+// gorilla/handlers.CompressHandler behavior this replaced.
+func negotiateEncoding(r *http.Request) string {
+	for enc := range strings.SplitSeq(r.Header.Get(acceptEncodingHeader), ",") {
+		enc = strings.TrimSpace(enc)
+		if enc == "gzip" || enc == "deflate" {
+			return enc
+		}
+	}
+	return ""
+}
+
+// compressHandler gzip/deflate-compresses eligible responses for clients
+// that advertise support via Accept-Encoding.
+//
+// A HEAD request is excluded outright: it has no body, and some handlers in
+// this codebase write their body payload unconditionally, relying on
+// net/http to discard it for HEAD at the transport level. Compressing here
+// would still spin up a compressor whose lone Close() trailer corrupts the
+// Content-Length net/http would otherwise report accurately. The
+// compressingResponseWriter below also defends against this more generally,
+// for any response that ends up writing no body at all, but excluding HEAD
+// up front avoids the sniffing overhead for a class of request that can
+// never benefit from it anyway.
+func compressHandler(next http.HandlerFunc) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		// Always add Accept-Encoding to Vary, to prevent intermediate caches
+		// from serving a compressed response to a client that can't decode
+		// it (or vice versa), regardless of what this particular request negotiates.
+		w.Header().Add("Vary", acceptEncodingHeader)
+
+		encoding := negotiateEncoding(r)
+		if encoding == "" || r.Method == http.MethodHead || r.Header.Get("Upgrade") != "" {
+			next.ServeHTTP(w, r)
+			return
+		}
+
+		cw := &compressingResponseWriter{ResponseWriter: w, encoding: encoding, statusCode: http.StatusOK}
+		next.ServeHTTP(cw, r)
+		cw.finish()
+	}
+}
+
+// compressingResponseWriter defers the compress-or-not decision until the
+// handler's first Write, so it can peek at the bytes actually being served:
+//
+//   - A Range/206 response is never compressed. http.ServeContent computes
+//     Content-Range and (per its own documented "always set Content-Length
+//     [for range requests]" rule) Content-Length from the uncompressed byte
+//     offsets; silently gzip'ing the body out from under that would make
+//     both headers describe bytes that were never sent.
+//   - Content that's already in a recognized compressed format (typically
+//     an OCI layer tarball) is passed through untouched.
+//
+// Because the decision -- and the resulting Content-Encoding/Content-Length
+// header changes -- only happens once real body bytes exist to inspect, a
+// response that never calls Write (including, redundantly with the HEAD
+// check above, any handler that legitimately writes nothing) never spins up
+// a compressor. That's what starves the bug described in compressHandler's
+// doc comment: gzip/deflate's harmless empty end-of-stream trailer has
+// nothing to masquerade as the whole response, because it's never written.
+type compressingResponseWriter struct {
+	http.ResponseWriter
+	encoding   string
+	statusCode int
+
+	wroteHeader bool
+	decided     bool
+	enc         io.WriteCloser // non-nil once compressing
+}
+
+func (cw *compressingResponseWriter) WriteHeader(code int) {
+	if cw.wroteHeader {
+		return
+	}
+	cw.wroteHeader = true
+	cw.statusCode = code
+}
+
+func (cw *compressingResponseWriter) Write(p []byte) (int, error) {
+	if !cw.decided {
+		cw.decided = true
+		if cw.statusCode != http.StatusPartialContent && !looksAlreadyCompressed(p) {
+			// Whatever Content-Length a handler (or http.ServeContent, for
+			// the uncompressed byte count) already set no longer applies
+			// once the body is compressed, and the final compressed size
+			// isn't known up front.
+			cw.Header().Del("Content-Length")
+			cw.Header().Set("Content-Encoding", cw.encoding)
+			if cw.encoding == "deflate" {
+				cw.enc, _ = flate.NewWriter(cw.ResponseWriter, gzip.DefaultCompression)
+			} else {
+				cw.enc, _ = gzip.NewWriterLevel(cw.ResponseWriter, gzip.DefaultCompression)
+			}
+		}
+		cw.ResponseWriter.WriteHeader(cw.statusCode)
+	}
+
+	if cw.enc != nil {
+		return cw.enc.Write(p)
+	}
+	return cw.ResponseWriter.Write(p)
+}
+
+// finish must be called after the wrapped handler returns, to flush a
+// still-pending WriteHeader call for a response that wrote no body (a
+// decision was never made, so there's nothing to compress), or to close out
+// the compressor -- emitting its end-of-stream trailer -- for one that did.
+func (cw *compressingResponseWriter) finish() {
+	if cw.enc != nil {
+		cw.enc.Close()
+		return
+	}
+	if cw.wroteHeader && !cw.decided {
+		cw.ResponseWriter.WriteHeader(cw.statusCode)
+	}
+}

ociserver/server.go

@@ -26,6 +26,9 @@ type ServerConfig struct {
 
 	// AuthMiddleware, when non-nil, is applied only to the built-in OCI routes.
 	AuthMiddleware func(http.Handler) http.Handler
+
+	// TransportCompression enables gzip and flate compression for GET responses
+	TransportCompression bool
 }
 
 // Server contains everything necessary to run the API portion of the service
@@ -96,7 +99,6 @@ func (s *Server) addRoutes() {
 	if len(s.cfg.Middlewares) > 0 {
 		r.Use(s.cfg.Middlewares...)
 	}
-
 	r.Route("/v2", func(r mux.Router) {
 		if s.cfg.AuthMiddleware != nil {
 			r.Use(s.cfg.AuthMiddleware)
@@ -113,13 +115,13 @@ func (s *Server) addRoutes() {
 		r.Delete("/*name/blobs/uploads/:session", s.blobUploadDelete())
 
 		// blobs
-		r.Head("/*name/blobs/:digest", s.blobHeadGet())
-		r.Get("/*name/blobs/:digest", s.blobHeadGet())
+		r.Head("/*name/blobs/:digest", s.checkTransportCompression(s.blobHeadGet()))
+		r.Get("/*name/blobs/:digest", s.checkTransportCompression(s.blobHeadGet()))
 		r.Delete("/*name/blobs/:digest", s.blobDelete())
 
 		// manifests
 		r.Head("/*name/manifests/:reference", s.manifestHeadGet())
-		r.Get("/*name/manifests/:reference", s.manifestHeadGet())
+		r.Get("/*name/manifests/:reference", s.checkTransportCompression(s.manifestHeadGet()))
 		r.Put("/*name/manifests/:reference", s.manifestPut())
 		r.Delete("/*name/manifests/:reference", s.manifestDelete())
 
@@ -173,3 +175,10 @@ type Redirecter interface {
 	// any error encountered while deciding.
 	Redirect(r *http.Request, desc oci.Descriptor, repo string) (bool, string, error)
 }
+
+func (s *Server) checkTransportCompression(next http.HandlerFunc) http.HandlerFunc {
+	if s.cfg.TransportCompression {
+		return compressHandler(next)
+	}
+	return next
+}

ociserver/server_test.go

@@ -2,11 +2,14 @@
 
 import (
 	"bytes"
+	"compress/gzip"
 	"context"
 	"errors"
+	"io"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -147,6 +150,33 @@
 	}`, rec.Body.String())
 }
 
+func TestGzipResponseWriterRemovesUncompressedContentLength(t *testing.T) {
+	body := []byte(`{"schemaVersion":2}`)
+	handler := compressHandler(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Length", strconv.Itoa(len(body)))
+		_, err := w.Write(body)
+		require.NoError(t, err)
+	})
+
+	req := httptest.NewRequest(http.MethodGet, "/v2/example/manifests/latest", nil)
+	req.Header.Set("Accept-Encoding", "gzip")
+	rec := httptest.NewRecorder()
+	handler(rec, req)
+
+	res := rec.Result()
+	defer res.Body.Close()
+	require.Empty(t, res.Header.Get("Content-Length"))
+	require.Equal(t, "gzip", res.Header.Get("Content-Encoding"))
+	require.Contains(t, res.Header.Values("Vary"), "Accept-Encoding")
+
+	gz, err := gzip.NewReader(res.Body)
+	require.NoError(t, err)
+	defer gz.Close()
+	got, err := io.ReadAll(gz)
+	require.NoError(t, err)
+	require.Equal(t, body, got)
+}
+
 func TestServerRejectsOverlongRepositoryName(t *testing.T) {
 	srv, err := New((*oci.Funcs)(nil), nil)
 	require.NoError(t, err)