Skip to content
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sei-cosmos/server/api/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,12 @@ func (s *Server) Start(cfg config.Config, apiMetrics *telemetry.Metrics) error {
if cfg.API.EnableUnsafeCORS {
allowAllCORS := handlers.CORS(handlers.AllowedHeaders([]string{"Content-Type"}))
s.mtx.Unlock()
return tmrpcserver.Serve(context.Background(), s.listener, allowAllCORS(h), tmCfg)
return tmrpcserver.Serve(context.Background(), s.listener, tmrpcserver.NewGzipHandler(allowAllCORS(h)), tmCfg)
}

logger.Info("starting API server...")
s.mtx.Unlock()
return tmrpcserver.Serve(context.Background(), s.listener, s.Router, tmCfg)
return tmrpcserver.Serve(context.Background(), s.listener, tmrpcserver.NewGzipHandler(s.Router), tmCfg)
}

// Close closes the API server.
Expand Down
2 changes: 1 addition & 1 deletion sei-tendermint/internal/inspect/rpc/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func Handler(rpcConfig *config.RPCConfig, routes core.RoutesMap) http.Handler {
if rpcConfig.IsCorsEnabled() {
rootHandler = addCORSHandler(rpcConfig, mux)
}
return rootHandler
return server.NewGzipHandler(rootHandler)
}

func addCORSHandler(rpcConfig *config.RPCConfig, h http.Handler) http.Handler {
Expand Down
1 change: 1 addition & 0 deletions sei-tendermint/internal/rpc/core/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ func (env *Environment) StartService(ctx context.Context, conf *config.Config) (
})
rootHandler = corsMiddleware.Handler(mux)
}
rootHandler = rpcserver.NewGzipHandler(rootHandler)
if conf.RPC.IsTLSEnabled() {
go func() {
if err := rpcserver.ServeTLS(
Expand Down
215 changes: 215 additions & 0 deletions sei-tendermint/rpc/jsonrpc/server/gzip.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
package server

import (
"bufio"
"compress/gzip"
"fmt"
"io"
"net"
"net/http"
"strconv"
"strings"
"sync"
)

const minCompressBytes = 1024

var gzPool = sync.Pool{
New: func() any {
w, _ := gzip.NewWriterLevel(io.Discard, gzip.BestSpeed)
return w
Comment thread
cursor[bot] marked this conversation as resolved.
},
}

type gzipResponseWriter struct {
resp http.ResponseWriter

gz *gzip.Writer
contentLength uint64
hasLength bool
inited bool
}

func (w *gzipResponseWriter) init() {
if w.inited {
return
}
w.inited = true

hdr := w.resp.Header()
length := hdr.Get("content-length")
if len(length) > 0 {
if n, err := strconv.ParseUint(length, 10, 64); err == nil {
w.hasLength = true
w.contentLength = n
}
}

// Skip compression if the inner handler already encoded the response or
// explicitly opted out via Transfer-Encoding: identity.
if hdr.Get("content-encoding") != "" || hdr.Get("transfer-encoding") == "identity" {
return
}

// Skip compression for small responses with a known Content-Length; below
// the threshold the gzip overhead outweighs the savings and the CPU cost
// per byte is not worth it for unauthenticated callers.
if w.hasLength && w.contentLength < minCompressBytes {
return
}

w.gz = gzPool.Get().(*gzip.Writer)
w.gz.Reset(w.resp)
hdr.Del("content-length")
hdr.Set("content-encoding", "gzip")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Setting Content-Encoding: gzip before the first write disables net/http's automatic Content-Type sniffing: with no explicit Content-Type, the server sniffs the compressed bytes and emits Content-Type: application/x-gzip instead of the real (uncompressed) type. Impact is low here because the Tendermint/Cosmos JSON-RPC handlers set Content-Type explicitly, but any handler relying on sniffing would be affected. Consider capturing/deriving Content-Type from the uncompressed bytes before enabling gzip. (Raised by Codex.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] Content-Type is not preserved before compression. Once Content-Encoding: gzip is set here and Write sends gzip bytes to the underlying ResponseWriter, net/http's content sniffing (triggered when the inner handler didn't set Content-Type explicitly) inspects the gzip magic bytes and emits Content-Type: application/x-gzip. Standard gzip middleware captures/sets the content type from the first pre-compression write (or calls http.DetectContentType on the uncompressed bytes) to avoid this. The wired JSON-RPC and gRPC-gateway handlers set application/json explicitly so impact is limited, but any handler relying on auto-detection will now serve a wrong Content-Type. (Raised by Codex P2.)

}
Comment thread
cursor[bot] marked this conversation as resolved.

func (w *gzipResponseWriter) Header() http.Header {
return w.resp.Header()
}

func (w *gzipResponseWriter) WriteHeader(status int) {
// Bodyless responses must not be compressed — gzip would write a stream
// terminator into what must be an empty body (RFC 7230 §3.3).
if status == http.StatusNoContent || status == http.StatusNotModified ||
Comment thread
amir-deris marked this conversation as resolved.
(status >= 100 && status <= 199) {
w.inited = true // skip gzip setup
w.resp.WriteHeader(status)
return
}
w.init()
w.resp.WriteHeader(status)
}

func (w *gzipResponseWriter) Write(b []byte) (int, error) {
w.init()

if w.gz == nil {
return w.resp.Write(b)
}

return w.gz.Write(b)
}

func (w *gzipResponseWriter) Flush() {
Comment thread
amir-deris marked this conversation as resolved.
// Decide the encoding before headers are committed: a bare Flush() before
// any Write() would otherwise commit identity-encoded headers, after which a
// later Write() sets Content-Encoding: gzip too late and emits gzip bytes
// under an identity encoding.
w.init()
if w.gz != nil {
_ = w.gz.Flush()
}
if f, ok := w.resp.(http.Flusher); ok {
f.Flush()
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flush blocked by statusWriter wrapper

Low Severity

gzipResponseWriter.Flush type-asserts w.resp as http.Flusher, but in every Serve/ServeTLS path resp is statusWriter, which embeds the http.ResponseWriter interface and does not implement Flusher. Compressed bytes therefore sit in the server buffer until the handler returns, despite the streaming-flush test and comments claiming incremental delivery works.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit affed4d. Configure here.


// Hijack implements http.Hijacker by forwarding to the inner ResponseWriter.
// The gzip writer is closed first so the hijacked connection starts clean.
func (w *gzipResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {
h, ok := w.resp.(http.Hijacker)
if !ok {
return nil, nil, fmt.Errorf("gzip middleware: underlying ResponseWriter does not implement http.Hijacker")
}
w.close()
return h.Hijack()
}

func (w *gzipResponseWriter) close() {
if w.gz == nil {
return
}
_ = w.gz.Close()
gzPool.Put(w.gz)
w.gz = nil
}

// acceptsGzip reports whether the request advertises gzip encoding support,
// respecting q-values and the "*" wildcard per RFC 7231 §5.3.4.
func acceptsGzip(r *http.Request) bool {
gzipQ := -1.0
starQ := -1.0
for _, field := range r.Header["Accept-Encoding"] {
for part := range strings.SplitSeq(field, ",") {
part = strings.TrimSpace(part)
coding, params, _ := strings.Cut(part, ";")
coding = strings.ToLower(strings.TrimSpace(coding))
q := 1.0
for p := range strings.SplitSeq(params, ";") {
p = strings.TrimSpace(p)
if k, v, ok := strings.Cut(p, "="); ok && strings.ToLower(strings.TrimSpace(k)) == "q" {
if f, err := strconv.ParseFloat(strings.TrimSpace(v), 64); err == nil {
q = f
}
}
}
switch coding {
case "gzip":
gzipQ = q
case "*":
starQ = q
}
}
}
if gzipQ >= 0 {
return gzipQ > 0
}
if starQ >= 0 {
return starQ > 0
}
return false
}

// ensureVaryAcceptEncoding appends Accept-Encoding to the Vary header exactly
// once, deduplicating against any value already set by the inner handler or
// CORS middleware. Vary: * already implies Accept-Encoding, so it is left as-is.
func ensureVaryAcceptEncoding(h http.Header) {
existing := h.Get("Vary")
for part := range strings.SplitSeq(existing, ",") {
v := strings.TrimSpace(part)
if strings.EqualFold(v, "Accept-Encoding") || v == "*" {
return
}
}
if existing == "" {
h.Set("Vary", "Accept-Encoding")
} else {
h.Set("Vary", existing+", Accept-Encoding")
}
}

// hasUpgradeToken reports whether the Upgrade header contains token (RFC 7230
// §6.7 permits a comma-separated list; each token is matched case-insensitively).
func hasUpgradeToken(r *http.Request, token string) bool {
for _, field := range r.Header["Upgrade"] {
for part := range strings.SplitSeq(field, ",") {
if strings.EqualFold(strings.TrimSpace(part), token) {
return true
}
}
}
return false
}

// NewGzipHandler wraps next with gzip response compression. Compression is
// skipped for clients that do not advertise gzip support via Accept-Encoding.
// WebSocket upgrades are passed through unmodified; gzipResponseWriter also
// implements http.Hijacker as defense-in-depth for non-canonical Upgrade values.
func NewGzipHandler(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Vary must be set on every response — compressed or not — so that CDN
// caches key on Accept-Encoding and never serve a wrong variant.
ensureVaryAcceptEncoding(w.Header())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] Vary: Accept-Encoding is set here before the inner handler runs. If a downstream handler or CORS middleware later calls w.Header().Set("Vary", ...) (rather than Add), it overwrites this value and drops Accept-Encoding. A CDN keyed only on the remaining Vary token could then serve a gzip-encoded variant to a client that never sent Accept-Encoding: gzip. Of the two wirings, rs/cors (env.go) uses Header().Add("Vary", ...) and is safe, but gorilla handlers.CORS (sei-cosmos server.go) and any future handler using Set are not guaranteed to be. Safer to (re)ensure the Vary token at header-commit time (in the wrapper's init()/WriteHeader) as well as up front. (Raised by Codex.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Vary: Accept-Encoding is set at handler entry, before the inner handler runs. An inner handler that does Header().Set("Vary", ...) would clobber it, letting a cache serve a compressed variant to a non-gzip client. The two CORS layers actually wired here (rs/cors and gorilla handlers.CORS) use Header().Add("Vary", ...), so the current wiring is safe — but this is fragile. Consider re-asserting Vary in init()/WriteHeader (when headers are committed) instead of at entry. (Raised by Codex P1; low practical impact given current wiring.)


if !acceptsGzip(r) || hasUpgradeToken(r, "websocket") {
next.ServeHTTP(w, r)
return
}

wrapper := &gzipResponseWriter{resp: w}
defer wrapper.close()
Comment thread
amir-deris marked this conversation as resolved.
Outdated
Comment thread
amir-deris marked this conversation as resolved.
Outdated

next.ServeHTTP(wrapper, r)
Comment thread
cursor[bot] marked this conversation as resolved.
})
}
Loading
Loading