58033 - multipart/form-data POST data should include charset for text controls

Status: VERIFIED DUPLICATE of bug 44464

(Core :: DOM: Core & HTML, defect, P3)

Description

[Adrian Havill]

HTML 4.01 says that all non-ASCII data should be POSTed via multipart/form-data
<URL:http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.2> and that
"User agents should supply the "Content-Type" header, accompanied by a "charset"
parameter." Lynx is one browser that does this.

This is necessary for server-side components that use a generic MIME processer
to process
non-ASCII data correctly.

Comment 1

[Adrian Havill]

Attachment: Writes a Content-Type line with a charset iff charset != us-ascii

Comment 2

[Gagan]

I agree with the overall idea of putting in the charset but nsFormFrame owners
and int'l folks should review this. ->pollmann(?)

Comment 3

[Eric Pollmann]

Attachment: updated patch (includes some suggested changes)

Comment 4

[Eric Pollmann]

Some suggested changes included in the above patch:

1) Add logic to increment the calculated content length
2) Remove conversion of charset to cstring and instead use EqualsWithConversion

I haven't really looked at this patch in depth to determine if it would affect
existing pages.  Adrian, can you clarify any thoughts on that?

I'm also not sure how we will ensure that if this case is hit we switch to
posting using multipart/form-data if we were in the middle of a
application/x-www-form-urlencoded post was that kind of switching what the spec
meant to happen?

Comment 5

[Adrian Havill]

Regarding how it would affect existing pages, I seriously doubt it would affect
pages in
the wild because multipart/form-data POSTs a) rare and b) when they are used,
they are used
exclusively for uploading files, because script authors do not like the
additional complexity of MIME parsing, and there is no (current) benefit of
using multipart/form-data over application/x-www-form-urlencoded (there would be
if the charset was present).

I don't understand what was meant by "switching" to multipart/form-data if we
were in the
middle of a application/x-www-formurlencoded.

While we're at it, we should add the "Content-Transfer-Encoding: binary", as
detailed in
the HTML 4.01 example for file upload, to the case where the component is a
file, so that
authors that decide to use a premade MIME component won't have problems with the
raw
8-bit data injected in the post causing the MIME parser to complain/fail.

Comment 6

[Eric Pollmann]

> I don't understand what was meant by "switching" to multipart/form-data if we
> were in the middle of a application/x-www-formurlencoded.

I think that was just me misunderstanding your earlier description.  :)  If I
understand correctly now, this fix should only change things if the form
encoding is set to multipart/form-data.

In that case I'd agree that the risk is pretty low - not to many people out
there using multipart/form-data for text input.. :)

Comment 7

[Eric Pollmann]

> While we're at it, we should add the "Content-Transfer-Encoding: binary", as
> detailed in the HTML 4.01 example for file upload. <snip>

This also sounds reasonable, though it would require quite a bit more testing -
it would be a miracle if we didn't hit a hand-rolled CGI unable to handle this
extra header before the data, but it could happen.  :)  Perhaps we should work
on this as a separate bug so that the two don't get tied together.

Comment 8

[Adrian Havill]

Content-Transfer-Encoding header suggestion moved to separate Bugzilla entry.
See:

<URL:http://bugzilla.mozilla.org/show_bug.cgi?id=58189>

Comment 9

[Adrian Havill]

After looking at the patch and the suggestion to use EqualsWithConversion, I
noticed the ifs are missing a bang "!" operator so the effect is opposite...
only forms in us-ascii will get the charset applied.

Comment 10

[Adrian Havill]

Attachment: fixed above suggested patch (refs to nsAutoString, us-ascii ignore logic)

Comment 11

[Dawn Endico]

this bug has a patch which needs review. adding keywords.

Comment 12

[Eric Pollmann]

Oops, this slid under the radar...  Thanks for the good catch (missing !).
r=pollmann for this change.  I'll try to get a super-review and then check in
this change.

Comment 13

[Marek Z. Jeziorek]

moving to 0.9 if we really have a patch. Please set target milestone to Future 
if we don't have a solution

Comment 14

[Adrian Havill]

don't use this version of the patch as the patch will need to be patched again
to handle the rest of the things that need to be in the multipart/form-data
according to HTML 4-- there's so much to multipart/form-data in the standards
that isn't supported that this is only the tip of the iceberg.

Use the latest patch in bug 44464.

For better or worse, it's macroized so you can remove (for those that hate
macros, by manually deleting the macro lines or those that like them, via
setting the functionality you want to true (1) or false (0)) the HTML 4
compliance that you don't want (for those concerned about the lack of testing
against interoperability with legacy non-HTML4.01/non-RFC2388 compliant
file-upload scripts)


*** This bug has been marked as a duplicate of 44464 ***

Comment 15

[James Green]

Verified dup.