Skip to content
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

net/http: NewFileTransport does not populate Response.Request on redirect #51562

Open
aronatkins opened this issue Mar 9, 2022 · 1 comment
Open
Labels
NeedsInvestigation

Comments

@aronatkins
Copy link

@aronatkins aronatkins commented Mar 9, 2022

What version of Go are you using (go version)?

$ go version
go version go1.17.8 darwin/amd64

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/aron/Library/Caches/go-build"
GOENV="/Users/aron/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/aron/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/aron/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.8"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/8j/pt4fjhkj11d3xd9344pdspmh0000gn/T/go-build2963024848=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

The http.NewFileTransport creates a transport where RoundTrip does not populate the Request field of its http.Response on redirects.

Given the following Go program:

package main

import (
	"fmt"
	"io"
	"net/http"
	"os"
)
func main() {
	transport := &http.Transport{}
	transport.RegisterProtocol("file", http.NewFileTransport(http.Dir("./www")))
	client := &http.Client{
		Transport: transport,
		CheckRedirect: func(req *http.Request, via []*http.Request) error {
			fmt.Printf("redirect-check: %s\n", req.URL)
			// we do our own redirect processing.
			return nil
		},
	}
	targetURL := "file:///sub"
	fmt.Printf("target url: %s\n", targetURL)
	resp, err := client.Get(targetURL)
	if err != nil {
		fmt.Printf("client error: %s\n")
		os.Exit(1)
	}

	if resp.Request != nil {
		fmt.Printf("request url: %s\n", resp.Request.URL)
	} else {
		fmt.Printf("no response request\n")
	}

	fmt.Printf("body-start\n")
	io.Copy(os.Stdout, resp.Body)
	fmt.Printf("body-end\n")
	
}

Using a local directory structure:

www/
    sub/
        index.html

What did you expect to see?

I expected the program to print:

target url: file:///sub
redirect-check: file:///sub/
request url: file:///sub/
body-start
<html><body>sub</body></html>
body-end

The request url print line indicates that the http.Response returned by the client has its Request field populated, which allows the caller to understand the final URL requested along the (local) redirect chain.

What did you see instead?

Using Go 1.17.8, the program prints:

$ go run main.go
target url: file:///sub
redirect-check: file:///sub/
no response request
body-start
<html><body>sub</body></html>
body-end

The no response redirect indicates that the http.Response returned by the client has an empty Request field, which is different than the behavior seen when using non-file transports.

@aronatkins
Copy link
Author

@aronatkins aronatkins commented Mar 9, 2022

As a workaround, use CheckRedirect to record the most-recent request; something like the following:

	var lastCheckedRequest *http.Request
	client := &http.Client{
		Transport: transport,
		CheckRedirect: func(req *http.Request, via []*http.Request) error {
			lastCheckedRequest = req
			return nil
		},
	}

You can then determine the final target URL using something like:

	// targetURL is where we landed after any redirects.
	targetURL := initialURL
	if r.Request != nil {
		targetURL = r.Request.URL.String()
	} else if lastCheckedRequest != nil {
		targetURL = lastCheckedRequest.URL.String()
	}

The lastCheckedRequest would need resetting between calls to the client and this approach does not handle concurrent client uses.

@seankhliao seankhliao added the NeedsInvestigation label Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

2 participants