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

Allow Content-Type in non-file parts of multipart #452

Closed
tmenier opened this issue Jun 5, 2019 · 1 comment
Closed

Allow Content-Type in non-file parts of multipart #452

tmenier opened this issue Jun 5, 2019 · 1 comment

Comments

@tmenier
Copy link
Owner

tmenier commented Jun 5, 2019

This is at least sort of a reversal of #392. We seem to have conflicting standards.

The HTML 5 standard says:

The parts of the generated url resource that correspond to non-file fields must not have a Content-Type header specified. Their names and values must be encoded using the character encoding selected above.

But RFC 7578 says:

Each part MAY have an (optional) "Content-Type" header field, which defaults to "text/plain".

Best explanation I could find is here:

The RFCs are designed to allow multipart/form-data to be usable in other contexts besides just HTML (though that is its most common use). In those other contexts, Content-Type is allowed. Just not in HTML 5 (but is allowed in HTML 4).

I'm inclined to think Flurl should favor the RFC for a couple reasons:

  1. Flurl is a "general" HTTP utility, and not specific to browser simulating. (That's certainly a use case, but secondary to talking to REST APIs.)

  2. Most servers looking to adhere to the HTML 5 spec will simply ignore the header. Flurl always sets Content-Type for multipart/formdata string fields, which is not standards-compliant. #392 was raised due to a bug in a specific server.

  3. Excluding the header is a legitimate hindrance to shortcut methods like AddJson being useful.

I haven't decided if this will simply be a reversal of #392 or if it will allow some sort of opt-in or opt-out of including headers. That's the most flexible obviously, but I'm considering whether this is even useful enough to justify the added complexity.

@tmenier
Copy link
Owner Author

tmenier commented Sep 26, 2020

As it turns out, it was easy to add flexibility here without adding complexity. AddString(s) already allows you to specify a content type; it'll just default to excluding the header entirely rather than adding one with text/plain. For AddJson and AddUrlEncoded, I'll add the headers back. You're not likely simulating an HTML form in those cases, so the requirement in the HTML 5 spec will hopefully not be applicable.

In order to generalize this change, I'm also making some subtle changes to the CapturedStringContent constructors that are technically breaking. .NET's StringContent doesn't support excluding the Content-Type header but Flurl's CapturedStringContent will. Here's the constructors:

CapturedStringContent(string) - created with default Content-Type header of text/plain; charset=UTF-8.

CapturedStringContent(string, string) - specify your own Content-Type header value, or pass null explicitly to exclude it.

Another subtlety here is you won't specify mediaType and encoding separately anymore like with StringContent. If you want to specify both, just pass the semicolon-delimited value exactly how it appears in the raw header. This is in line with Flurl's (opinionated) notion of headers as simple name/value pairs.

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

No branches or pull requests

1 participant