Introducing query string normalizer which fixes retarded php query string handling. #15
Conversation
httpx/middleware/query_normalizer.go
Outdated
|
|
||
| // WithQueryNormalizer fixes wrong php query string handling as array | ||
| func WithQueryNormalizer(h http.Handler) http.Handler { | ||
| rxp, e := regexp.Compile("\\[[0-9]+\\]") |
httpx/middleware/query_normalizer.go
Outdated
| } | ||
|
|
||
| func getNormalizedValue(key string, qVal []string, rxp *regexp.Regexp) (string, []string) { | ||
| isNastyArray := rxp.Match([]byte(key)) |
There was a problem hiding this comment.
I would think twice about using Regex here. It is very expensive.
httpx/middleware/query_normalizer.go
Outdated
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| q := r.URL.Query() | ||
|
|
||
| normalizedQuery := make(url.Values, 0) |
There was a problem hiding this comment.
Either use url.Values{} or optimise for the expected length. The latter is preferable.
httpx/middleware/query_normalizer.go
Outdated
| func WithQueryNormalizer(h http.Handler) http.Handler { | ||
| rxp, e := regexp.Compile("\\[[0-9]+\\]") | ||
| if e != nil { | ||
| panic(e) |
There was a problem hiding this comment.
Is panicing here a good idea?
There was a problem hiding this comment.
The only reason why there can be error is badly created regexp. If so, then why not panic?
There was a problem hiding this comment.
I suppose it is ok in this case, but rather use MustCompile.
httpx/middleware/query_normalizer.go
Outdated
| }) | ||
| } | ||
|
|
||
| func getNormalizedValue(key string, qVal []string, rxp *regexp.Regexp) (string, []string) { |
There was a problem hiding this comment.
Are you normalising the value or the key?
There was a problem hiding this comment.
i think normizeKey would be better
| func WithQueryNormalizer(h http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| if i := strings.Index(r.URL.RawQuery, "["); i == -1 { | ||
| return |
There was a problem hiding this comment.
You need to delegate the request before returning
httpx/middleware/query_normalizer.go
Outdated
| // WithQueryNormalizer fixes wrong php query string handling as array | ||
| func WithQueryNormalizer(h http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| if i := strings.Index(r.URL.RawQuery, "["); i == -1 { |
httpx/middleware/query_normalizer.go
Outdated
| }) | ||
| } | ||
|
|
||
| func getNormalizedValue(key string) string { |
There was a problem hiding this comment.
naming: You are not normalising a value. Call it normalizeQueryKey instead
httpx/middleware/query_normalizer.go
Outdated
| } | ||
|
|
||
| q := r.URL.Query() | ||
| normalizedQuery := make(url.Values, 0) |
There was a problem hiding this comment.
pessimistically assign the map size
There was a problem hiding this comment.
You could use len(q), but for small maps it would make very little to no difference. If we needlessly allocated more space than needed, it could get, in fact, slower. I would leave it up to @rafalmnich .
There was a problem hiding this comment.
Not changing anything in benchmarks with even for query with many changes like:
/?param[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val¶m[smth][0]=zero¶m[smth][0][val][1][asdf][4][asetrhyty][5][ewrytu][7][aerterwert][7]=val
httpx/middleware/query_normalizer.go
Outdated
| q := r.URL.Query() | ||
| normalizedQuery := make(url.Values, 0) | ||
|
|
||
| for key, qVal := range q { |
There was a problem hiding this comment.
naming: qVal should be vals
No description provided.