handle_empty_content! causes problems with Go's http.Client

[catatsuy]

refs: #1603

Go's http.client's automatic redirection uses the same Content-Type for GET. This request is invalid in rack 2.2.

I don't know if the Go implementation is wrong or the rack implementation is wrong. But because it is used in the actual implementation, could you please consider an option to disable it?

For example, you can see it in the following code.

require 'sinatra'

post '/file_upload' do
  redirect '/redirect', 302
end

get '/redirect' do
  return 'abcd'
end

package main

import (
	"bytes"
	"io"
	"log"
	"mime/multipart"
	"net/http"
	"os"
)

func main() {
	hClient := &http.Client{
		CheckRedirect: func(req *http.Request, via []*http.Request) error {
			return nil
			// If you enable the following code, you will not get the error.
			// return http.ErrUseLastResponse
		},
	}

	file, err := os.Open("test.png")
	if err != nil {
		log.Fatal(err)
	}
	defer file.Close()

	body := &bytes.Buffer{}
	writer := multipart.NewWriter(body)
	part, err := writer.CreateFormFile("image", "upload.png")
	if err != nil {
		log.Fatal(err)
	}

	_, err = io.Copy(part, file)
	if err != nil {
		log.Fatal(err)
	}

	contentType := writer.FormDataContentType()

	err = writer.Close()
	if err != nil {
		log.Fatal(err)
	}

	req, err := http.NewRequest(http.MethodPost, "http://localhost:4567/file_upload", body)
	if err != nil {
		log.Fatal(err)
	}

	req.Header.Set("Content-Type", contentType)

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

[jeremyevans]

Rack 3 will support raising Rack::Multipart::EmptyContentError in this case, so you could rescue that error and handle it specifically. Currently, you'll have to rescue EOFError to handle it. Go is violating RFC 2046 section 5.1, which states The body must then contain one or more body parts (i.e. empty bodies are not allowed), assuming it is submitting an empty body with a multipart/form-data content type. You should probably raise this issue with the Go developers, and point out that it violates the RFC.

I suppose I'm not against adding an option that does this automatically, so long as it is off by default. @ioquatix what are your thoughts about that?

[ioquatix]

@jeremyevans based on your assessment, Go is wrong and I don't think we should bother dealing with this. As you state, in Rack 3.0 we raise an exception which can be handled if applications need to work around this issue.

[catatsuy]

@jeremyevans @ioquatix I registered this issue for the Go issue but it was closed. I share it with you. golang/go#52519

[jeremyevans]

@catatsuy Thanks for sharing that. Apparently the Go developer who closed the issue doesn't understand the problem, since it is not related to redirects specifically. The Go developer is stating you should use 307 instead of 302 in your example code, to avoid triggering the buggy behavior in Go. That may work around the bug in Go, but it doesn't explain why Go is submitting invalid requests.

[ioquatix]

Here is a followup PR against golang: golang/go#57273.