Repository navigation
Conversation
bindData indexed inputValue[0] on the scalar-map paths and on the
tagged-struct paths without checking the length, so a header whose value
slice was nil or empty panicked with "index out of range [0] with length 0"
instead of binding nothing. BindHeaders passes the request header map
straight to bindData, so BindHeaders inherited the panic.
Guard both reads. An empty slice now behaves like an absent key, while
[]string{""} - a present but empty value - still binds, so the check keys
on length rather than on the value being empty.
Closes labstack#3169
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
BindHeaderspanics when a header's value slice is nil or empty:BindHeaderspasses the request header map straight tobindData, which readsv[0]on the scalar-map paths (bind.go:245,bind.go:249) andinputValue[0]on the tagged-struct paths (
bind.go:337,bind.go:364) without checking thelength.
Header["X-Empty"] = nilor[]string{}therefore panics instead ofbinding nothing.
Note this is not reachable from a raw incoming HTTP request —
net/httpneverproduces an empty value slice — so it is not a remote denial of service. It is
reachable for any in-process caller that builds or mutates a
Headermap, andfor anything calling
bindDatawith a map it controls.Change
Guard both reads. An empty value slice is treated like an absent key.
[]string{""}— a present but empty value — still binds, so the check keys onlength rather than on the value being empty.
TestBindHeadersEmptyStringValueStillBindspins that distinction.
Testing
Four tests in
bind_empty_header_test.go:TestBindHeadersEmptyValueSlice— nil and[]string{}into a tagged struct,as subtests, since they are distinct states reaching the same panic.
TestBindHeadersEmptyValueSliceIntoMap— the scalar-map destination path.TestBindHeadersPresentValueStillBinds— the guard does not swallow a real value.TestBindHeadersEmptyStringValueStillBinds—[]string{""}still binds.With
bind.goreverted and the tests kept, the run fails with the exact panicabove. With the change, all four pass and the full root suite stays green
(
ok github.com/labstack/echo/v5), including the 86 existing binding tests.gofmt -lis clean on both changed files;go vetis clean.echo.goandmiddleware/decompress.goare flagged bygofmton master and are untouched here.Closes #3169