mirror of
https://github.com/caddyserver/caddy.git
synced 2026-10-31 00:10:10 -04:00
* perf: bypass header key normalization for canonical literals
http.Header's Get, Set, Add and Del run textproto.CanonicalMIMEHeaderKey
on every call. When the key is a canonical literal the result is the key
itself, so the scan is pure overhead. Access the map directly instead.
Only hot, allocation-free paths are converted, and only where the key is
a canonical literal. Call sites taking a key from config, a template or
the request itself still go through the normalizing methods, since they
need it for correctness.
allHeaderValues and lastHeaderValue dropped their http.CanonicalHeaderKey
call for the same reason: all callers pass canonical literals.
benchstat, Apple M5 Pro, go1.27.1:
PrepareRequest-15 659.4n ± 2% -> 457.7n ± 2% -30.58% (p=0.000 n=12)
AcceptedEncodings-15 330.8n ± 5% -> 309.4n ± 7% -6.44% (p=0.019 n=10)
ResponseWriterWriteHeader/event-stream-15 1007.5n ± 3% -> 732.5n ± 1% -27.30%
ResponseWriterWriteHeader/informational-15 60.0n ± 3% -> 48.0n ± 2% -20.01%
ResponseWriterWriteHeader/not-modified-15 214.3n ± 6% -> 175.3n ± 4% -18.20%
ResponseWriterWriteHeader/connect-15 60.2n ± 3% -> 51.9n ± 4% -13.85%
ResponseWriterWriteHeader/html-15 206.5n ± 4% -> 197.9n ± 2% -4.19%
geomean -14.38%
Allocations are unchanged.
* tests: verify http.Header map keys are canonical
Indexing http.Header directly skips the canonicalization Get/Set/Add/Del
perform, which also puts the key outside what CI checks. canonicalheader
only inspects method calls. staticcheck's SA1008 covers reads, but stops
descending as soon as an assignment writes to a header, so it misses the
write, the delete builtin and the read-modify-write append. A key that is
not canonical in those positions is a silent lookup miss, not a compile
error.
Walk the loaded packages and verify every string constant used as a key in
an http.Header map operation matches http.CanonicalHeaderKey. A helper that
takes the key from its caller would hide the literal again, so a parameter
named with a canonical prefix opts its arguments into the same check.
The WebSocket headers Caddy deliberately writes with RFC 6455 casing are
allowed explicitly; they are only ever written out, never looked up against
a parsed header map.
Covers 64 operations, runs in under a second, and promotes golang.org/x/tools
from an indirect to a direct dependency at the version already in the module
graph.
* headercheck: resolve aliases of http.Header
* headercheck: check http.Header composite literal keys
* headercheck: note overlap with canonicalheader
* headercheck: drop parameter-name check
Helpers now take the indexed values, so every key sits in an index expression the analyzer and the linter already check.
* headercheck: scope RFC 6455 exceptions to their call sites
A key-based allow-list also accepted reads such as h["Sec-WebSocket-Key"], which miss because parsed headers are canonical. Honor //nolint:canonicalheader on the line instead, which the linter will use too.
* encode: read every Accept-Encoding field line
AcceptedEncodings read only the first field line, so a client that split
the list across several lines had the rest ignored. RFC 9110 section 5.3
treats repeated list-field lines as one comma-joined list. Iterate over
the items of every line instead of joining them, and skip empty items,
which used to yield an empty encoding. file_server's precompressed
selection uses the same function and is fixed with it.
Fixes https://github.com/caddyserver/caddy/issues/8272
Co-authored-by: Kağan Küçük <kaankck@gmail.com>
* encode: read every Cache-Control and Content-Encoding field line
Only the first field line was read, but RFC 9110 section 5.3 makes
multiple lines of a list-based field equivalent to one comma-joined
value:
- Cache-Control: no-transform on a later line was ignored, so the
request or response was compressed anyway.
- Content-Encoding: an empty first line followed by a real coding let
the handler encode the body a second time.
* encode: parse Cache-Control no-transform per RFC 9111
The substring match missed No-Transform, though RFC 9111 section 5.2 compares directive names case-insensitively, and fired on text that is not the directive: longer tokens such as x-no-transform-ext, quoted arguments such as community="no-transform", and semicolon-separated values, which are not valid Cache-Control syntax. Split the field on commas outside quoted strings and match only a well-formed no-transform element.
* encode: keep accepted encoding preferences on the stack
AcceptedEncodings counted commas to size the preference slice, and the variable capacity put it on the heap. A constant capacity covers typical Accept-Encoding headers and lets the compiler keep the slice on the stack; append still moves it to the heap for longer ones. BenchmarkAcceptedEncodings: 151.8ns -> 120.5ns (-21%), 192 -> 64 B/op, 2 -> 1 allocs/op (n=8).
* encode: build the encoded ETag without fmt
fmt.Sprintf boxed both arguments and parsed the format on every encoded response with a strong ETag. Concatenation: 47ns -> 16.5ns, 56 -> 24 B, 3 -> 1 allocs.
* reverseproxy: access response Server and Via headers directly
finalizeResponse runs on every proxied response; skip the key normalization as prepareRequest already does on the request side.
* encode: access headers directly in Write, WriteHeader and Close
These run once per response; convert the remaining literal-key Get, Set and Add calls like the rest of the handler.
* encode: match Cache-Control no-transform with httpguts
httpguts.HeaderValuesContainsToken already compares list elements case-insensitively across field lines, so the hand-written list parser is unnecessary. A no-transform inside a comma-separated quoted argument now also disables encoding, which only errs toward not transforming.
* encode: inline the no-transform check
isEncodeAllowed only wrapped one httpguts call. Its unit test now drives the response directive through ServeHTTP instead.
---------
Co-authored-by: Kağan Küçük <kaankck@gmail.com>
180 lines
5.4 KiB
Go
180 lines
5.4 KiB
Go
// Copyright 2015 Matthew Holt and The Caddy Authors
|
|
//
|
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
|
// you may not use this file except in compliance with the License.
|
|
// You may obtain a copy of the License at
|
|
//
|
|
// http://www.apache.org/licenses/LICENSE-2.0
|
|
//
|
|
// Unless required by applicable law or agreed to in writing, software
|
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
// See the License for the specific language governing permissions and
|
|
// limitations under the License.
|
|
|
|
// Package headercheck verifies that every http.Header key written directly
|
|
// into the map uses net/http's canonical form.
|
|
//
|
|
// Hot paths index http.Header directly to skip the canonicalization that
|
|
// Get/Set/Add/Del perform on every call. That trades a linted call for an
|
|
// unlinted map operation: the canonicalheader linter only inspects method
|
|
// calls, and staticcheck's SA1008 stops descending as soon as an assignment
|
|
// writes to a header, so neither covers a direct write or a delete. A key
|
|
// that is not canonical there is a silent lookup miss, not a compile error.
|
|
//
|
|
// This covers four shapes the enabled linters miss:
|
|
//
|
|
// h["x-key"] = []string{"v"} // direct write
|
|
// http.Header{"x-key": {"v"}} // composite literal
|
|
// delete(h, "x-key") // delete builtin
|
|
// h["x-key"] = append(h["x-key"], "v) // read-modify-write
|
|
package headercheck
|
|
|
|
import (
|
|
"go/ast"
|
|
"go/constant"
|
|
"go/token"
|
|
"go/types"
|
|
"net/http"
|
|
"strings"
|
|
"testing"
|
|
|
|
"golang.org/x/tools/go/packages"
|
|
)
|
|
|
|
// headerType is the type whose map operations must use canonical keys.
|
|
const headerType = "net/http.Header"
|
|
|
|
// nolintDirective marks a deliberate non-canonical key on its line, such as
|
|
// the RFC 6455 spellings written for case-sensitive upstreams.
|
|
const nolintDirective = "//nolint:canonicalheader"
|
|
|
|
// TODO: delete this package once golangci-lint ships https://github.com/lasiar/canonicalheader/pull/90;
|
|
// the linter honors the same //nolint:canonicalheader directives.
|
|
func TestHeaderKeysAreCanonical(t *testing.T) {
|
|
cfg := &packages.Config{
|
|
Mode: packages.NeedName | packages.NeedSyntax | packages.NeedTypes |
|
|
packages.NeedTypesInfo | packages.NeedDeps | packages.NeedImports,
|
|
Dir: "../..",
|
|
Tests: false,
|
|
}
|
|
|
|
pkgs, err := packages.Load(cfg, "./...")
|
|
if err != nil {
|
|
t.Fatalf("loading packages: %v", err)
|
|
}
|
|
if packages.PrintErrors(pkgs) > 0 {
|
|
t.Fatal("packages failed to load; see errors above")
|
|
}
|
|
|
|
var checked int
|
|
for _, pkg := range pkgs {
|
|
for _, file := range pkg.Syntax {
|
|
checked += checkFile(t, pkg, file)
|
|
}
|
|
}
|
|
|
|
// Guard against the walk silently covering nothing, e.g. if package
|
|
// loading is reconfigured and stops yielding type information.
|
|
if checked == 0 {
|
|
t.Fatal("no http.Header map operations found; the check is not running")
|
|
}
|
|
t.Logf("checked %d direct http.Header map operations", checked)
|
|
}
|
|
|
|
// checkFile reports the number of direct http.Header map operations examined.
|
|
func checkFile(t *testing.T, pkg *packages.Package, file *ast.File) int {
|
|
var checked int
|
|
|
|
exempt := make(map[int]bool)
|
|
for _, group := range file.Comments {
|
|
for _, c := range group.List {
|
|
if strings.HasPrefix(c.Text, nolintDirective) {
|
|
exempt[pkg.Fset.Position(c.Slash).Line] = true
|
|
}
|
|
}
|
|
}
|
|
|
|
report := func(pos token.Pos, key, form string) {
|
|
canonical := http.CanonicalHeaderKey(key)
|
|
if key == canonical || exempt[pkg.Fset.Position(pos).Line] {
|
|
return
|
|
}
|
|
t.Errorf("%s: %s uses non-canonical key %q; want %q",
|
|
pkg.Fset.Position(pos), form, key, canonical)
|
|
}
|
|
|
|
// keyOf returns the constant string value of an index or argument
|
|
// expression, so both literals and named constants are covered.
|
|
keyOf := func(expr ast.Expr) (string, bool) {
|
|
tv, ok := pkg.TypesInfo.Types[expr]
|
|
if !ok || tv.Value == nil || tv.Value.Kind() != constant.String {
|
|
return "", false
|
|
}
|
|
return constant.StringVal(tv.Value), true
|
|
}
|
|
|
|
isHeader := func(expr ast.Expr) bool {
|
|
tv, ok := pkg.TypesInfo.Types[expr]
|
|
if !ok || tv.Type == nil {
|
|
return false
|
|
}
|
|
named, ok := types.Unalias(tv.Type).(*types.Named)
|
|
if !ok {
|
|
return false
|
|
}
|
|
obj := named.Obj()
|
|
return obj != nil && obj.Pkg() != nil &&
|
|
obj.Pkg().Path()+"."+obj.Name() == headerType
|
|
}
|
|
|
|
ast.Inspect(file, func(n ast.Node) bool {
|
|
switch node := n.(type) {
|
|
// h["Key"], including the assignment form h["Key"] = ...
|
|
// that SA1008 skips.
|
|
case *ast.IndexExpr:
|
|
if !isHeader(node.X) {
|
|
return true
|
|
}
|
|
key, ok := keyOf(node.Index)
|
|
if !ok {
|
|
return true
|
|
}
|
|
checked++
|
|
report(node.Index.Pos(), key, "http.Header index")
|
|
|
|
// http.Header{"Key": ...}, which stores the key verbatim.
|
|
case *ast.CompositeLit:
|
|
if !isHeader(node) {
|
|
return true
|
|
}
|
|
for _, elt := range node.Elts {
|
|
kv, ok := elt.(*ast.KeyValueExpr)
|
|
if !ok {
|
|
continue
|
|
}
|
|
key, ok := keyOf(kv.Key)
|
|
if !ok {
|
|
continue
|
|
}
|
|
checked++
|
|
report(kv.Key.Pos(), key, "http.Header literal")
|
|
}
|
|
|
|
// delete(h, "Key"), which no enabled linter inspects.
|
|
case *ast.CallExpr:
|
|
if ident, ok := node.Fun.(*ast.Ident); ok && ident.Name == "delete" && len(node.Args) == 2 {
|
|
if _, ok := pkg.TypesInfo.Uses[ident].(*types.Builtin); ok && isHeader(node.Args[0]) {
|
|
if key, ok := keyOf(node.Args[1]); ok {
|
|
checked++
|
|
report(node.Args[1].Pos(), key, "delete on http.Header")
|
|
}
|
|
}
|
|
}
|
|
}
|
|
return true
|
|
})
|
|
|
|
return checked
|
|
}
|