internal/functions: make snippets func more robust

Use the context from incoming requests to do outgoing requests.
This way, if a client gives up or disconnects, we give up too.

Refuse any incoming request that isn't a GET or a POST.

Set up the outgoing POST request body when building the request.
Setting http.Request.Body directly after the constructor seems OK,
but the constructor also performs some of its documented logic.

Defer closing the incoming body; the net/http docs say we should.
It's unclear what happens if we don't, and it seemed to mostly work,
but there's no reason to go against the documentation.

In the success case, don't simply copy the body over;
also copy over the response status code.
Otherwise, if the server responds with a 403 or 500 and no body,
we might give the client a 200 with no body.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: Ia9a9633c34d9dd6d81de6e747aca36bbe9c48bdf
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cuelang.org/+/1171059
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Paul Jolly <[email protected]>
Dispatch-Trailer: {"type":"trybot","CL":1171222,"patchset":1,"ref":"refs/changes/22/1171222/1","targetBranch":"master"}

internal/functions/snippets/snippets.go

@@ -35,47 +35,53 @@ type Function struct {
 
 // ServeHTTP is the implementation of the snippets serverless function.
 func (fn Function) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	ctx := r.Context()
 	if fn.DevelopmentMode {
 		w.Header().Set("Access-Control-Allow-Origin", "*")
 	}
 	f := func(format string, args ...interface{}) {
 		fmt.Fprintf(w, format, args...)
 	}
-	client := &http.Client{}
-	if r.Method == "POST" {
+	switch r.Method {
+	case "POST":
 		// Share
 		url := "https://play.golang.org/share"
-		req, err := http.NewRequest("POST", url, nil)
+		req, err := http.NewRequestWithContext(ctx, "POST", url, r.Body)
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 			f("Failed to create onwards GET URL: %v", err)
 			return
 		}
 		req.Header.Add("User-Agent", userAgent)
-		req.Body = r.Body
-		resp, err := client.Do(req)
+		resp, err := http.DefaultClient.Do(req)
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 			f("Failed in onward request: %v", err)
 			return
 		}
+		defer resp.Body.Close()
+		w.WriteHeader(resp.StatusCode)
 		io.Copy(w, resp.Body)
-	} else {
+	case "GET":
 		// Retrieve via the parameter id
-		url := fmt.Sprintf("https://play.golang.org/p/%v.go", r.FormValue("id"))
-		req, err := http.NewRequest("GET", url, nil)
+		url := fmt.Sprintf("https://play.golang.org/p/%s.go", r.FormValue("id"))
+		req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 			f("Failed to create onwards GET URL: %v", err)
 			return
 		}
 		req.Header.Add("User-Agent", userAgent)
-		resp, err := client.Do(req)
+		resp, err := http.DefaultClient.Do(req)
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 			f("Failed in onward request: %v", err)
 			return
 		}
+		defer resp.Body.Close()
+		w.WriteHeader(resp.StatusCode)
 		io.Copy(w, resp.Body)
+	default:
+		http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
 	}
 }