Allow URLs up to 32KB and improve parsing speed (#20874)

Before/after...
```
Benchmark_hasBadPathComponent/long-32          	   43936	     27232 ns/op	 146.89 MB/s	   32768 B/op	       1 allocs/op
Benchmark_hasBadPathComponent/long-32          	   89956	     13375 ns/op	 299.07 MB/s	       0 B/op	       0 allocs/op
```

* Remove unused.
This commit is contained in:
Klaus Post 2025-01-27 08:42:45 -08:00 committed by GitHub
parent c5d19ecebb
commit dcc000ae2c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 56 additions and 17 deletions

View File

@ -22,11 +22,11 @@ import (
"net" "net"
"net/http" "net/http"
"path" "path"
"path/filepath"
"runtime/debug" "runtime/debug"
"strings" "strings"
"sync/atomic" "sync/atomic"
"time" "time"
"unicode"
"github.com/dustin/go-humanize" "github.com/dustin/go-humanize"
"github.com/minio/minio-go/v7/pkg/s3utils" "github.com/minio/minio-go/v7/pkg/s3utils"
@ -293,12 +293,6 @@ func parseAmzDateHeader(req *http.Request) (time.Time, APIErrorCode) {
return time.Time{}, ErrMissingDateHeader return time.Time{}, ErrMissingDateHeader
} }
// Bad path components to be rejected by the path validity handler.
const (
dotdotComponent = ".."
dotComponent = "."
)
func hasBadHost(host string) error { func hasBadHost(host string) error {
if globalIsCICD && strings.TrimSpace(host) == "" { if globalIsCICD && strings.TrimSpace(host) == "" {
// under CI/CD test setups ignore empty hosts as invalid hosts // under CI/CD test setups ignore empty hosts as invalid hosts
@ -311,21 +305,41 @@ func hasBadHost(host string) error {
// Check if the incoming path has bad path components, // Check if the incoming path has bad path components,
// such as ".." and "." // such as ".." and "."
func hasBadPathComponent(path string) bool { func hasBadPathComponent(path string) bool {
if len(path) > 4096 { n := len(path)
// path cannot be greater than Linux PATH_MAX if n > 32<<10 {
// this is to avoid a busy loop, that can happen // At 32K we are beyond reasonable.
// if the caller sends path of following style
// a/a/a/a/a/a/a/a...
return true return true
} }
path = filepath.ToSlash(strings.TrimSpace(path)) // For windows '\' must be converted to '/' i := 0
for _, p := range strings.Split(path, SlashSeparator) { // Skip leading slashes (for sake of Windows \ is included as well)
switch strings.TrimSpace(p) { for i < n && (path[i] == SlashSeparatorChar || path[i] == '\\') {
case dotdotComponent: i++
}
for i < n {
// Find the next segment
start := i
for i < n && path[i] != SlashSeparatorChar && path[i] != '\\' {
i++
}
// Trim whitespace of segment
segmentStart, segmentEnd := start, i
for segmentStart < segmentEnd && unicode.IsSpace(rune(path[segmentStart])) {
segmentStart++
}
for segmentEnd > segmentStart && unicode.IsSpace(rune(path[segmentEnd-1])) {
segmentEnd--
}
// Check for ".." or "."
switch {
case segmentEnd-segmentStart == 2 && path[segmentStart] == '.' && path[segmentStart+1] == '.':
return true return true
case dotComponent: case segmentEnd-segmentStart == 1 && path[segmentStart] == '.':
return true return true
} }
i++
} }
return false return false
} }

View File

@ -22,6 +22,7 @@ import (
"net/http/httptest" "net/http/httptest"
"net/url" "net/url"
"strconv" "strconv"
"strings"
"testing" "testing"
"github.com/minio/minio/internal/crypto" "github.com/minio/minio/internal/crypto"
@ -184,3 +185,27 @@ func TestSSETLSHandler(t *testing.T) {
} }
} }
} }
func Benchmark_hasBadPathComponent(t *testing.B) {
tests := []struct {
name string
input string
want bool
}{
{name: "empty", input: "", want: false},
{name: "backslashes", input: `\a\a\ \\ \\\\\\\`, want: false},
{name: "long", input: strings.Repeat("a/", 2000), want: false},
{name: "long-fail", input: strings.Repeat("a/", 2000) + "../..", want: true},
}
for _, tt := range tests {
t.Run(tt.name, func(b *testing.B) {
b.SetBytes(int64(len(tt.input)))
b.ReportAllocs()
for i := 0; i < b.N; i++ {
if got := hasBadPathComponent(tt.input); got != tt.want {
t.Fatalf("hasBadPathComponent() = %v, want %v", got, tt.want)
}
}
})
}
}