-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Path stripping #3
Conversation
main.go
Outdated
@@ -86,18 +82,33 @@ func main() { | |||
s.ListenAndServe() | |||
} | |||
|
|||
func newProxy(backend *url.URL, l log.Logger) *httputil.ReverseProxy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally prefer having the loggers, db and such as first arguments. Not sure what the convention is (if there is any).
Also, some of your functions get the logger first (NewAllowedActions
for example). So consistency could be nice :)
main.go
Outdated
proxy := httputil.NewSingleHostReverseProxy(backend) | ||
proxy.ErrorLog = stdlog.New(log.NewStdlibAdapter(l), "", stdlog.LstdFlags) | ||
proxy.Transport = &http.Transport{ | ||
DisableCompression: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious, why disable compression ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes little sense for binary, is all.
main.go
Outdated
|
||
s := &http.Server{ | ||
Addr: fmt.Sprintf(":%d", listenPort), | ||
Handler: decorateHandler(proxy, rlBucket), | ||
Handler: decorateHandler(proxy, rlBucket, logger), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
decorateHandler
could accept logger in first position too
main.go
Outdated
// Defining early needed handlers last | ||
decorators = append( | ||
decorators, | ||
handlers.NewIgnoreFaviconRequests(), | ||
handlers.NewRateLimitHandler(b, logger), | ||
handlers.NewRateLimitHandler(b, l), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could accept logger first as well
This feature allows you to strip a part of the path being sent to Imaginary.
An example:
With:
Sents the path
/enlarge
to Imaginary, instead of/static/enlarge
This is useful when dealing with (e.g.) GKE's L7 firewalls