Skip to content
This repository has been archived by the owner on Mar 29, 2023. It is now read-only.

multifilereader: fix multipart/form-data #8

Closed
wants to merge 1 commit into from

Conversation

MichaelMure
Copy link

According to https://tools.ietf.org/html/rfc7578#section-4.2, in a Content-Disposition header
of a part in a multipart/form-data body:

  • the disposition must be of type form-data
  • a name parameter (not explicitely specified by the RFC, but I suppose non-empty)

As this name parameter is unused by IPFS, this commit simply generate a placeholder with
a counter, to conform to the HTTP specification.

According to https://tools.ietf.org/html/rfc7578#section-4.2, in a Content-Disposition header
of a part in a multipart/form-data body:

- the disposition must be of type `form-data`
- a `name` parameter (not explicitely specified by the RFC, but I suppose non-empty)

As this `name` parameter is unused by IPFS, this commit simply generate a placeholder with
a counter, to conform to the HTTP specification.
@MichaelMure
Copy link
Author

Those deviations from the RFS break the parsing of multipart/form-data requests (say, ipfs add) with common HTTP library, one of them being golang's http package.

@MichaelMure
Copy link
Author

CI failure is unrelated to this PR.

mpWriter *multipart.Writer
closed bool
mutex *sync.Mutex
fieldCounter int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go fmt ./...

@@ -88,7 +89,10 @@ func (mfr *MultiFileReader) Read(buf []byte) (written int, err error) {
// write the boundary and headers
header := make(textproto.MIMEHeader)
filename := url.QueryEscape(path.Join(path.Join(mfr.path...), entry.Name()))
header.Set("Content-Disposition", fmt.Sprintf("file; filename=\"%s\"", filename))
name := fmt.Sprintf("data%d", mfr.fieldCounter)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do browser set this to when user picks multiple files in file input? Can one name repeat?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://tools.ietf.org/html/rfc7578#section-4.3

To match widely deployed implementations, multiple files MUST be sent by supplying each file in a separate part but all with the same "name" parameter.

I suppose we want to mimic a form with multiple upload from the same field, so I should change that.

@Stebalien
Copy link
Member

Fixed in #14. Thanks for finding this!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants