internal/https: fix race condition in ListenAndServe
There was a race condition when the opt.AutocertCacheBucket field was
specified. The handler = http.HandlerFunc(redirectToHTTPS) line could
run before (or after) the value of handler was used in the following
line that did serveAutocertTLS(handler, opt.AutocertCacheBucket).
As a result, it was possible for the HTTPS server to end up using the
redirectToHTTPS handler, instead of the intended original handler
parameter passed to the ListenAndServe function.
Change-Id: I95967eed1176f236a87131d0c054ded680886591
Reviewed-on: https://go-review.googlesource.com/c/159697
Reviewed-by: Andrew Bonventre <andybons@golang.org>
diff --git a/internal/https/https.go b/internal/https/https.go
index e0d119f..c63280d 100644
--- a/internal/https/https.go
+++ b/internal/https/https.go
@@ -49,10 +49,14 @@
errc := make(chan error)
if ln != nil {
go func() {
+ var h http.Handler
if opt.AutocertCacheBucket != "" {
- handler = http.HandlerFunc(redirectToHTTPS)
+ // handler is served primarily via HTTPS, so just redirect HTTP to HTTPS.
+ h = http.HandlerFunc(redirectToHTTPS)
+ } else {
+ h = handler
}
- errc <- fmt.Errorf("http.Serve = %v", http.Serve(ln, handler))
+ errc <- fmt.Errorf("http.Serve = %v", http.Serve(ln, h))
}()
}
if opt.AutocertCacheBucket != "" {
