pkg/tool/http: allow configuration of following redirects

The default net/http.Client in Go follows redirects. This is a sensible
default, both in Go and in the pkg/tool/http Do wrappers.

However there are times when it is useful to be able to assert the
actual status code returned by an endpoint _is_ a redirect, which
otherwise cannot be determined when redirects are automatically
followed.

We fix this by introducing a new top-level config field followRedirects
on http.Do, which defaults to true, but can explicitly be set to false
in which case the status code, body etc of the first response is used to
populate the response field.

Fixes #3896.

Signed-off-by: Paul Jolly <[email protected]>
Change-Id: I7788414b60c50831e25b24453e5a85d290f477f1
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1213859
Reviewed-by: Roger Peppe <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: myitcv

pkg/tool/http/http.cue

@@ -1,11 +1,11 @@
 // Copyright 2018 The CUE Authors
-// 
+//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
-// 
+//
 //     http://www.apache.org/licenses/LICENSE-2.0
-// 
+//
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -25,6 +25,10 @@ Do: {
 	method: string
 	url:    string // TODO: make url.URL type
 
+	// followRedirects controls whether the http client follows redirects
+	// or not. Defaults to true, like the default net/http client in Go.
+	followRedirects: *true | bool
+
 	tls: {
 		// Whether the server certificate must be validated.
 		verify: *true | bool

pkg/tool/http/http.go

@@ -116,6 +116,29 @@ func (c *httpCmd) Run(ctx *task.Context) (res interface{}, err error) {
 		// TODO: timeout
 	}
 
+	// Rather clumsily, we need to also default followRedirects here because
+	// it's still valid for tasks to be specified via the special $id field, in
+	// which case we cannot be clear that the documented CUE-based defaults have
+	// been applied.
+	//
+	// This is noted as something to fix, more precisely a mistake not to make
+	// again, in https://cuelang.org/issue/1325
+	followRedirects := true
+	followRedirectsValue := ctx.Obj.LookupPath(cue.MakePath(cue.Str("followRedirects")))
+	if followRedirectsValue.Exists() {
+		var err error
+		followRedirects, err = followRedirectsValue.Bool()
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if !followRedirects {
+		client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
+			return http.ErrUseLastResponse
+		}
+	}
+
 	req, err := http.NewRequest(method, u, r)
 	if err != nil {
 		return nil, err

pkg/tool/http/http_test.go

@@ -163,3 +163,104 @@ func TestParseHeaders(t *testing.T) {
 		})
 	}
 }
+
+// TestRedirect exercises the followRedirects configuration on an http.Do request
+func TestRedirect(t *testing.T) {
+	mux := http.NewServeMux()
+
+	// In this test server, /a redirects to /b. /b serves "hello"
+	mux.Handle("/a", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, "/b", http.StatusFound)
+	}))
+	mux.Handle("/b", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprint(w, "hello")
+	}))
+
+	server := httptest.NewUnstartedServer(mux)
+	server.Start()
+	t.Cleanup(server.Close)
+
+	testCases := []struct {
+		name            string
+		path            string
+		statusCode      int
+		followRedirects *bool
+		body            *string
+	}{
+		{
+			name:       "/a silent on redirects",
+			path:       "/a",
+			statusCode: 200,
+			body:       ref("hello"),
+		},
+		{
+			name:            "/a with explicit followRedirects: true",
+			path:            "/a",
+			statusCode:      200,
+			followRedirects: ref(true),
+			body:            ref("hello"),
+		},
+		{
+			name:            "/a with explicit followRedirects: false",
+			path:            "/a",
+			statusCode:      302,
+			followRedirects: ref(false),
+		},
+		{
+			name:       "/b silent on redirects",
+			path:       "/b",
+			statusCode: 200,
+			body:       ref("hello"),
+		},
+		{
+			name:            "/b with explicit followRedirects: true",
+			path:            "/b",
+			statusCode:      200,
+			followRedirects: ref(true),
+			body:            ref("hello"),
+		},
+		{
+			name:            "/b with explicit followRedirects: false",
+			path:            "/b",
+			statusCode:      200,
+			followRedirects: ref(true),
+			body:            ref("hello"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			v3 := parse(t, "tool/http.Get", fmt.Sprintf(`
+			{
+				url: "%s%s"
+			}`, server.URL, tc.path))
+
+			if tc.followRedirects != nil {
+				v3 = v3.FillPath(cue.ParsePath("followRedirects"), *tc.followRedirects)
+			}
+
+			resp, err := (*httpCmd).Run(nil, &task.Context{Obj: v3})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// grab the response
+			response := resp.(map[string]any)["response"].(map[string]any)
+
+			if got := response["statusCode"]; got != tc.statusCode {
+				t.Fatalf("status not as expected: wanted %d, got %d", got, tc.statusCode)
+			}
+
+			if tc.body != nil {
+				want := *tc.body
+				if got := response["body"]; got != want {
+					t.Fatalf("body not as expected; wanted %q, got %q", got, want)
+				}
+			}
+		})
+	}
+}
+
+func ref[T any](v T) *T {
+	return &v
+}