americanas-tech / restQL-golang

Microservice query language and platform
http://restql.b2w.io/
MIT License
17 stars 8 forks source link

fix(restQL): header comparison must be case insensitive #36

Closed wedneyyuri closed 3 years ago

wedneyyuri commented 4 years ago

This is required to be compliant with the HTTP/1.1 spec

Reference: https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/header_casing#:~:text=When%20handling%20HTTP%2F1.1%2C%20Envoy,rely%20on%20specific%20header%20casing.

caiorcferreira commented 4 years ago

Thanks, this is a nice corner case to catch!

My only suggestion is to used strings.EqualFold instead of strings.ToLower as it is more efficient, and strings operations in Go in a hot path like this code can have a significant impact on the GC pressure under high load.

Reference on the strings.EqualFold: https://www.digitalocean.com/community/questions/how-to-efficiently-compare-strings-in-go

wedneyyuri commented 4 years ago

Hi @caiorcferreira thanks for your suggestion, but I don't see a way to use strings.EqualFold here. In this snippet code we're not looping over the keys of disallowedHeaders.

caiorcferreira commented 4 years ago

Sorry, I should have been more clear. In this case, changing disallowedHeaders to a slice and looping over it using strings.EqualFold should beat strings.ToLower overall, since each time it's called it will perform an allocation since strings are immutable in Go, even with the added time complexity because, usually, m >>> n, where m is the quantity of request headers and n is the quantity of disallowedHeaders.

wedneyyuri commented 3 years ago

Thanks for your support, I made the requested changes :v:

caiorcferreira commented 3 years ago

Nice catch! Thanks for your contribution.