Skip to content

refactor: improve internal interface names#2318

Merged
erikdubbelboer merged 12 commits into
valyala:masterfrom
Harshal96:feat/fix-issue-2229
Jul 5, 2026
Merged

refactor: improve internal interface names#2318
erikdubbelboer merged 12 commits into
valyala:masterfrom
Harshal96:feat/fix-issue-2229

Conversation

@Harshal96

Copy link
Copy Markdown
Contributor

Fixes #2229

Harshal96 and others added 5 commits July 3, 2026 13:02
The fuzzer compares the parsed host against net/url and lowercases the
input to hide that fasthttp normalizes the host while net/url does not.
That trick cannot reach bytes hidden behind percent-encoding: "%4c"
survives ToLower and only decodes to 'L' afterwards, so fasthttp
returns "l" where net/url keeps "L". Found by the cifuzz job on an
unrelated pull request.

Lower the expectation instead, byte-wise via lowercaseBytes, since
strings.ToLower would mangle non-UTF-8 bytes like a decoded "%80".
Both hosts are added as seeds.
* perf: avoid redundant scans when parsing request headers

RequestHeader.parse walks the header block twice before the lines are
actually parsed: readRawHeaders looks for the end of the block, then
headerScanner searches for the CRLFCRLF terminator once more. The
scanner now reuses the block end that readRawHeaders already found,
guarded by a check that the block really ends in CRLFCRLF. Blocks with
bare LF terminators fail that check and take the old search path, so
those odd cases keep behaving exactly as before.

While at it:

- trim header values with a plain loop, bytes.TrimLeft rebuilds its
  ASCII set on every call for the " \t" cutset
- drop the bytes.IndexByte(key, ' ') checks in front of
  normalizeHeaderKey, it already refuses keys with invalid bytes
  such as spaces
- simplify isValidHeaderKey, the noCanon flag changed nothing

benchstat (Apple M2 Pro, n=10):

RequestHeaderRead-12    79.48n ± 1%   63.22n ± 3%   -20.46% (p=0.000)
ServerGet1ReqPerConn-12 1.693µ ± 3%   1.671µ ± 1%   ~ (p=0.123)

* perf: reuse scanner work when parsing header lines

readContinuedLineSlice already scans every header line for the colon,
so hand that position to next() instead of finding it again with
bytes.Cut. Lines handed out there can never start with a space or tab
(the first line is rejected by the scanner, later ones are joined into
the previous header as continuations), so trimming never shifts the
position.

isValidHeaderKey now also reports whether the key contains a space that
survives trailing-whitespace trimming. The per-byte key loops in
parseHeaders and parseTrailer only existed to derive exactly that flag,
their reject branches were unreachable since the scanner validates
first. The response quirk that a space in a key closes the connection
is kept.

readLine loses its searchStart loop: the scanner truncates the block at
the terminator, so every line is guaranteed to end in \n.

benchstat (Apple M2 Pro, n=10), on top of the previous commit:

RequestHeaderRead-12    63.22n ± 3%   56.06n ± 1%   -11.33% (p=0.000)

and against master:

RequestHeaderRead-12    79.48n ± 1%   56.06n ± 1%   -29.48% (p=0.000)
valyala#2311)

When os.Rename fails (e.g., /tmp and destination on different filesystems),
the copy fallback path leaves the original multipart temp file orphaned
in /tmp. Add defer os.Remove(ff.Name()) to clean it up after copying.

@erikdubbelboer erikdubbelboer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please rebase this on master.

Comment thread streaming.go Outdated
)

type headerInterface interface {
type requestStreamHeader interface {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This could use a better name, this interface is used for response streams too. Maybe just bodyStreamHeader?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Harshal96 and others added 7 commits July 4, 2026 15:47
…sure (valyala#2302)

* fix(pprofhandler): use exact path matching to prevent debug data exposure

PprofHandler uses bytes.HasPrefix for routing, which means
/debug/pprof/cmdlineFoo matches the cmdline handler. This leaks
process command-line arguments to any path that starts with a valid
pprof endpoint prefix.

Replace HasPrefix with matchPprofPath, which requires an exact match
(or exact match + trailing slash, matching net/http/pprof's
DefaultServeMux behaviour). This prevents /debug/pprof/cmdlineFoo,
/debug/pprof/profileX, etc. from serving sensitive debug data.

Fixes valyala#2258

* fix(pprofhandler): resolve lint issues — remove unused var, fix gci formatting, remove builtin shadow

- Remove unused pprofPrefix variable (gci unused linter)
- Fix gci import formatting in pprof.go and pprof_test.go
- Remove custom min() function that shadows predeclared builtin (gocritic builtinShadowDecl)
- Add missing trailing newline in pprof.go

Addresses maintainer review feedback on valyala#2302.

* fix(pprofhandler): return 404 for paths with trailing slash

net/http/pprof does not register handlers for paths ending in /, so
requesting /debug/pprof/heap/ returns 404 via the default mux.

Update matchPprofPath to use strict equality only (no trailing slash
matching) and add an early 404 check in PprofHandler for any path
ending in /.

Renames test from AcceptsTrailingSlash to RejectsTrailingSlash and
updates matchPprofPath test to expect false for trailing slash.

---------

Co-authored-by: xbrxr03 <[email protected]>
@erikdubbelboer erikdubbelboer merged commit 26cd2d1 into valyala:master Jul 5, 2026
11 checks passed
@erikdubbelboer

Copy link
Copy Markdown
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: improve internal interface naming (headerInterface, connTLSer, connKeepAliveer)

6 participants