net/http: request.Method is wrong when path start by //

[eraac]

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

$ go version
go version go1.11.4 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/kevin/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/kevin/code/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.4/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/kevin/code/go/src/gitlab.com/cyclesinfos/consumer/station-state/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/43/45xhjfkn0xbbr4flsgzh879h0000gn/T/go-build866475413=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

send post request with url start with two // (tbh it's was a mistake)

What did you expect to see?

server receive a post request

What did you see instead?

server receive a get request

Code

package main

import (
	"fmt"
	"net/http"
	"net/http/httputil"
	"time"
)

func main() {
	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		fmt.Printf("method: %s\n", r.Method)
	})

	go func(){_ = http.ListenAndServe(":8080", http.DefaultServeMux)}()

	client := http.Client{Timeout: time.Second}
	req, _ := http.NewRequest(http.MethodPost, "http://localhost:8080//route", nil)
	_, _ = client.Do(req)

	time.Sleep(time.Millisecond * 500)

	bs, _ := httputil.DumpRequest(req, false)
	fmt.Printf("\n----\n%s", string(bs))
}

Output

method: GET

----
POST //route HTTP/1.1
Host: localhost:8080

[agnivade]

/cc @bradfitz

[cuonglm]

@eraac With this patch:

diff --git a/src/net/http/server.go b/src/net/http/server.go
index aa9c3f5d2e..88f53ff5ac 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -2336,7 +2336,11 @@ func (mux *ServeMux) Handler(r *Request) (h Handler, pattern string) {
                _, pattern = mux.handler(host, path)
                url := *r.URL
                url.Path = path
-               return RedirectHandler(url.String(), StatusMovedPermanently), pattern
+               status := StatusMovedPermanently
+               if r.Method != "GET" || r.Method != "HEAD" {
+                       status = StatusTemporaryRedirect
+               }
+               return RedirectHandler(url.String(), status), pattern
        }
 
        return mux.handler(host, r.URL.Path)

You will get expected result.

But IMHO, we should return 500 error with a message like redirect with form data without user interaction won't be perform. HTTP spec requires response to a request other than GET or HEAD, the user agent MUST NOT automatically redirect the request unless it can be confirmed by the user

How do you think @bradfitz ?

[kshitij10496]

I studied how other CLI HTTP clients (like curl and http) behave in comparison to the default HTTP Client provided by Go's standard library for the same HTTP server.

server.go

package main

import (
	"fmt"
	"log"
	"net/http"
	"net/http/httputil"
)

func main() {
	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
                // debug incoming requests
		reqd, err := httputil.DumpRequest(r, false)
		if err != nil {
			log.Println(err)
		}
		fmt.Printf("\n[request]: %s\n", reqd)

		w.WriteHeader(http.StatusOK)
	})
	fmt.Println("starting server on http://localhost:8080")
	if err := http.ListenAndServe(":8080", http.DefaultServeMux); err != nil {
		log.Fatal(err)
	}
}

---

1. curl

• Request

$ curl -v -X POST http://localhost:8080//route
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8080 (#0)
> POST //route HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 301 Moved Permanently
< Location: /route
< Date: Wed, 06 Mar 2019 16:47:09 GMT
< Content-Length: 0
<
* Connection #0 to host localhost left intact

• Serve Log is empty!

---

2. http

• Request

$ http -v POST :8080//route                      
POST //route HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Content-Length: 0
Host: localhost:8080
User-Agent: HTTPie/0.9.9



HTTP/1.1 301 Moved Permanently
Content-Length: 0
Date: Wed, 06 Mar 2019 16:57:28 GMT
Location: /route

• Serve Log is empty!

---

client.go

package main

import (
	"fmt"
	"log"
	"net/http"
	"net/http/httputil"
	"time"
)

func main() {
	client := http.Client{Timeout: time.Second}
	req, err := http.NewRequest(http.MethodPost, "http://localhost:8080//route", nil)
	if err != nil {
		log.Fatal(err)
	}

	res, err := client.Do(req)
	if err != nil {
		log.Fatal(err)
	}

	reqd, err := httputil.DumpRequestOut(req, false)
	if err != nil {
		log.Println(err)
	}
	fmt.Printf("\n[request]: %s\n", reqd)

	resd, err := httputil.DumpResponse(res, false)
	if err != nil {
		log.Println(err)
	}
	fmt.Printf("\n[response]: %s\n", resd)
}

• Server Log:

[request]: GET /route HTTP/1.1
Host: localhost:8080
Accept-Encoding: gzip
Referer: http://localhost:8080//route
User-Agent: Go-http-client/1.1

• Client Log:

[request]: POST //route HTTP/1.1
Host: localhost:8080
User-Agent: Go-http-client/1.1
Content-Length: 0
Accept-Encoding: gzip



[response]: HTTP/1.1 200 OK
Date: Wed, 06 Mar 2019 17:07:47 GMT
Content-Length: 0

---

The reason this happens could be due to following the internally-generated redirect for the initial request POST //route and somehow landing at GET /route.
As @Gnouc astutely pointed out above, RFC 2616 Sec 10.3.2 clearly states that no automatic redirect take place in such cases.

"If the 301 status code is received in response to a request other than GET or HEAD, the user agent MUST NOT automatically redirect the request unless it can be confirmed by the user, since this might change the conditions under which the request was issued. "

Thoughts @bradfitz ?

[kshitij10496]

I was reading more about this today and I found that we are redirecting all HTTP requests (except "HEAD" requests) using a new "GET" request via redirectBehaviour.

The default policy of the HTTP Client is to redirect automatically and stop after 10 successive redirects.

RFC 7231 categorically explains the use of GET method for automatic redirection:

Note: For historical reasons, a user agent MAY change the request
method from POST to GET for the subsequent request. If this
behavior is undesired, the 307 (Temporary Redirect) status code
can be used instead.

As a result, we have to explicitly restrict the client from automatically following any redirects by via CheckRedirect while initialising the HTTP Client. I defer to this SO question with regards to the implementation: How Can I Make the Go HTTP Client NOT Follow Redirects Automatically?

After this research, I do not think of this issue as a bug/unexpected behaviour but rather a conscious decision from the team with proper documentation. Based on the practical nature of the use case and the buzz on SO around this question, the only suggestion I have would be to empower the end-users with a suitable interface for creating such no-redirect-following clients from the standard library package itself. This would just be a syntactic sugar - another level of abstraction. I defer to the opinion of far more experienced engineers here. 😄

cc @bradfitz @odeke-em

net/http/client.go

var NoRedirectFollow = func(req *http.Request, via []*http.Request) error {
        return http.ErrUseLastResponse
}

end-user/client.go

myClient := &http.Client{CheckRedirect: http.NoRedirectFollow}

References:

1. net/http: regression in client redirect logic #18570
2. Documentation for the above behaviour in Do function
3. Redirect Implementation comments
4. RFC 7231 - 301 Moved Permanently Section