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

content-disposition is optional for multipart/related #175

Open
franher opened this issue May 16, 2017 · 8 comments
Open

content-disposition is optional for multipart/related #175

franher opened this issue May 16, 2017 · 8 comments

Comments

@franher
Copy link

franher commented May 16, 2017

According https://tools.ietf.org/html/rfc2387#section-4 on multipart/related content-disposition header is optional.

Library only grab a file if content-disposition exists.

I think library should be able to handle files without this optional header and save it with a random (based on timestamp for example) filename.

@dougwilson
Copy link
Contributor

Pull requests welcome!

@franher
Copy link
Author

franher commented May 16, 2017

hello @dougwilson,

I'm figuring out how to distinguish between a field or a file. My initial point should be use the Content-Type to be sure we are facing a file. What do you think?

@dougwilson
Copy link
Contributor

I'm not sure. I would expect it to distinguish them however the spec says.

@dougwilson
Copy link
Contributor

This is the spec this module is trying to adhere to: https://tools.ietf.org/html/rfc7578 and it states (in https://tools.ietf.org/html/rfc7578#section-4.2):

4.2. Content-Disposition Header Field for Each Part

Each part MUST contain a Content-Disposition header field [RFC2183]
where the disposition type is "form-data". The Content-Disposition
header field MUST also contain an additional parameter of "name"; the
value of the "name" parameter is the original field name from the
form (possibly encoded; see Section 5.1).

I haven't really looked at the one you linked to, but figured to state what this module is trying to do may help here.

@franher
Copy link
Author

franher commented May 16, 2017

The spec you mention is for multipart/form-data.

In talking about multipart/related, that looking in the code could be supported https://github.com/pillarjs/multiparty/blob/master/index.js#L40

for multipart/related, content-disposition is optional.

@dougwilson
Copy link
Contributor

Right, it is for multipart/form-data. The first line in the README is as follows:

Parse http requests with content-type multipart/form-data, also known as file uploads.

I honestly had no idea this module allowed multipart/related. Seems weird because they seems to have different requirements in some areas. I don't know much about multipart/related so will defer to you on what changes need to be made in that regard, but they shouldn't break the multipart/form-data compliance of this module, at least.

@dougwilson
Copy link
Contributor

I just noticed this module seems to have zero tests for multipart/related. Perhaps we should just release a new major just removing that content-type? Or are the similar enough to support both? Can you help me understand multipart/related ? How does it related to multipart/form-data ; like what are the similarities and the differences.

If both are supported, that would help, since afaik there is just a single parsing code path, so I assume it is just parsing multipart/related as if it was multipart/form-data, so writing down what the differences are between the two will help understand what is broken today and how much of a different code path would need to be added.

@franher
Copy link
Author

franher commented May 16, 2017

of course, I want to keep the behaviour of this module for multipart/form-data.

When I faced the issue of looking for multipart/related parsers, I couldn't find even a module supporting that.

According multipart/related spec, it's useful when the body parts are related between them.

The Multipart/Related media type is intended for compound objects
consisting of several inter-related body parts. For a
Multipart/Related object, proper display cannot be achieved by
individually displaying the constituent body parts. The content-type
of the Multipart/Related object is specified by the type parameter.
The "start" parameter, if given, points, via a content-ID, to the
body part that contains the object root. The default root is the
first body part within the Multipart/Related body.

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

No branches or pull requests

2 participants