According to the doc on client.Do, a 307 or 308 redirect preserves the original
http method and body provided that the request.GetBody function is defined
My thought then is that access to reads and writes on the req.Body must
be serialized to avoid a race condition.
Using a mutex for this prevented the race conditions.
My own question then is, what is the implication of serializing such access.
Wouldn't that have been the implementation if the stdlib was to make the
req.Body
safe during a redirect?
- Uzondu
> On 5 Mar 2019, at 3:23 PM, virra...@gmail.com wrote:
>
> I've been playing with server which redirects requests but then I noticed
> weird races.
> What is odd that if these races are expected it would mean that using `*File`
> in `req.Body`, for requests which are redirected, is not safe - this is
> because we have races between `Read` and `Close`.
>
> I've run following script with: `go run -race concept.go`
>
> package main
>
> import (
> "crypto/rand"
> "fmt"
> "io"
> "io/ioutil"
> "net/http"
> "sync"
> "time"
> )
>
> const (
> addr1 = "localhost:8060"
> addr2 = "localhost:8061"
> )
>
> type customReader struct {
> iter int
> size int
> }
>
> func (r *customReader) Read(b []byte) (int, error) {
> maxRead := r.size - r.iter
> if maxRead == 0 {
> return 0, io.EOF
> }
> if maxRead > len(b) {
> maxRead = len(b)
> }
> n, err := rand.Read(b[:maxRead])
> r.iter += maxRead
> return n, err
> }
>
> func (r *customReader) Close() error {
> // Uncomment this for even more races :((
> // r.iter = 0
> return nil
> }
>
> func (r *customReader) Reset() {
> r.iter = 0
> }
>
> func second(w http.ResponseWriter, r *http.Request) {
> ioutil.ReadAll(r.Body)
> }
>
> func first(w http.ResponseWriter, r *http.Request) {
> http.Redirect(w, r, "http://"+addr2+"/second";,
> http.StatusTemporaryRedirect)
> }
>
> func main() {
> mux1 := http.NewServeMux()
> mux1.HandleFunc("/first", first)
> srv1 := http.Server{
> Addr: addr1,
> Handler: mux1,
> }
> go func() {
> err := srv1.ListenAndServe()
> fmt.Printf("err: %v", err)
> }()
>
> mux2 := http.NewServeMux()
> mux2.HandleFunc("/second", second)
> srv2 := http.Server{
> Addr: addr2,
> Handler: mux2,
> }
> go func() {
> err := srv2.ListenAndServe()
> fmt.Printf("err: %v", err)
> }()
>
> time.Sleep(time.Second)
> client := http.DefaultClient
>
> wg := &sync.WaitGroup{}
> for i := 0; i < 1000; i++ {
> reader := &customReader{size: 900000}
> wg.Add(1)
> go func(r *customReader) {
> for {
> req, err := http.NewRequest(http.MethodPut,
> "http://"+addr1+"/first";, r)
> if err != nil {
> fmt.Printf("%v", err)
> }
> req.GetBody = func() (io.ReadCloser, error) {
> return &customReader{size: 900000}, nil
> }
>
> if resp, err := client.Do(req); err != nil {
> // fmt.Printf("error: %v", err)
> } else if resp.StatusCode >=
> http.StatusBadRequest {
> // fmt.Printf("status code: %d",
> resp.StatusCode)
> }
>
> // Reset reader and try to reuse it in next
> request
> r.Reset()
> }
>
> wg.Done()
> }(reader)
> }
>
> wg.Wait() // infinite wait
>
> srv1.Close()
> srv2.Close()
> }
>
>
>
> Races which are occuring:
>
> ==================
> WARNING: DATA RACE
> Write at 0x00c0003e00a0 by goroutine 87:
> main.main.func3()
> /home/januszm/concept.go:43 +0x55
>
> Previous write at 0x00c0003e00a0 by goroutine 3899:
> [failed to restore the stack]
>
> Goroutine 87 (running) created at:
> main.main()
> /home/januszm/concept.go:91 +0x3b0
>
> Goroutine 3899 (finished) created at:
> net/http.(*Transport).dialConn()
> /usr/local/go/src/net/http/transport.go:1358 +0xb89
> net/http.(*Transport).getConn.func4()
> /usr/local/go/src/net/http/transport.go:1015 +0xd0
> ==================
> ==================
> WARNING: DATA RACE
> Write at 0x00c0002ca750 by goroutine 488:
> main.main.func3()
> /home/januszm/concept.go:43 +0x55
>
> Previous write at 0x00c0002ca750 by goroutine 1951:
> main.(*customReader).Read()
> /home/januszm/concept.go:33 +0x156
> net/http.transferBodyReader.Read()
> /usr/local/go/src/net/http/transfer.go:62 +0x77
> io.copyBuffer()
> /usr/local/go/src/io/io.go:402 +0x143
> net/http.(*transferWriter).writeBody()
> /usr/local/go/src/io/io.go:364 +0x76e
> net/http.(*Request).write()
> /usr/local/go/src/net/http/request.go:655 +0x7c3
> net/http.(*persistConn).writeLoop()
> /usr/local/go/src/net/http/transport.go:1961 +0x321
>
> Goroutine 488 (running) created at:
> main.main()
> /home/januszm/concept.go:91 +0x3b0
>
> Goroutine 1951 (running) created at:
> net/http.(*Transport).dialConn()
> /usr/local/go/src/net/http/transport.go:1358 +0xb89
> net/http.(*Transport).getConn.func4()
> /usr/local/go/src/net/http/transport.go:1015 +0xd0
> ==================
>
> When L39 is uncommented, we also get:
>
> ==================
> WARNING: DATA RACE
> Write at 0x00c0001620e0 by goroutine 58:
> main.(*customReader).Close()
> /home/januszm/concept.go:39 +0x3a
> net/http.(*Client).do()
> /usr/local/go/src/net/http/request.go:1346 +0x6f2
> main.main.func3()
> /usr/local/go/src/net/http/client.go:509 +0x183
>
> Previous write at 0x00c0001620e0 by goroutine 245:
> main.(*customReader).Read()
> /home/januszm/concept.go:33 +0x156
> net/http.transferBodyReader.Read()
> /usr/local/go/src/net/http/transfer.go:62 +0x77
> io.copyBuffer()
> /usr/local/go/src/io/io.go:402 +0x143
> net/http.(*transferWriter).writeBody()
> /usr/local/go/src/io/io.go:364 +0x76e
> net/http.(*Request).write()
> /usr/local/go/src/net/http/request.go:655 +0x7c3
> net/http.(*persistConn).writeLoop()
> /usr/local/go/src/net/http/transport.go:1961 +0x321
>
> Goroutine 58 (running) created at:
> main.main()
> /home/januszm/concept.go:87 +0x3b0
>
> Goroutine 245 (running) created at:
> net/http.(*Transport).dialConn()
> /usr/local/go/src/net/http/transport.go:1358 +0xb89
> net/http.(*Transport).getConn.func4()
> /usr/local/go/src/net/http/transport.go:1015 +0xd0
> ==================
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-nuts+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
