Bug 32140

Summary: REGRESSION(r50072): Mailman administrative functionality is broken
Product: WebKit Reporter: William Siegrist <wsiegrist>
Component: New BugsAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, darin, dbates, mrowe, webkit.review.bot
Priority: P2 Keywords: HasReduction, InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.6   
URL: http://src.macosforge.org/webkit-post-data-bug-test.php
Attachments:
Description Flags
proposed fix darin: review+

Description William Siegrist 2009-12-03 19:10:45 PST
Reduced case: http://src.macosforge.org/webkit-post-data-bug-test.php

If you check a box and hit the corresponding submit button, you'll notice the checkbox name "abc%40def" is encoded as "abc%2540def" only when multipart encoding is used.  I would expect the field name to be the same in both cases. Firefox 3.5.5 and Safari 4.0.4 do not encode the name. I can reproduce with nightly r51580.
Comment 1 Mark Rowe (bdash) 2009-12-03 19:15:41 PST
This regressed in <http://trac.webkit.org/changeset/50072>.  It breaks administrative functionality within the Mailman interface that is used on lists.webkit.org.
Comment 2 Mark Rowe (bdash) 2009-12-03 19:32:20 PST
Why did we choose to URL encode field names within these headers?  The only thing that RFC 2388, the multipart/form-data specification, has to say about encoding of field names is:

>    Field names originally in non-ASCII character sets may be encoded
>    within the value of the "name" parameter using the standard method
>   described in RFC 2047.

RFC 2047, Message Header Extensions for Non-ASCII Text, appears to discuss the use of quoted-printable encoding.  The cases we are concerned about a clearly not non-ASCII, but this does seem to directly conflict with our decision to URL encode the field names.
Comment 3 Mark Rowe (bdash) 2009-12-03 19:32:42 PST
<rdar://problem/7443458>
Comment 4 Alexey Proskuryakov 2009-12-03 22:15:06 PST
Percent escaping was chosen randomly, we can definitely try another encoding form. Encoding the percent sign itself was a poorly thought through last minute change, and it can be just removed from the set of encoded characters for an easy fix.

I did not know about RFC 2388, and I do not know if anyone follows it.
Comment 5 Alexey Proskuryakov 2009-12-10 15:53:24 PST
Created attachment 44644 [details]
proposed fix
Comment 6 WebKit Review Bot 2009-12-10 15:57:06 PST
style-queue ran check-webkit-style on attachment 44644 [details] without any errors.
Comment 7 Mark Rowe (bdash) 2009-12-10 16:17:36 PST
Comment on attachment 44644 [details]
proposed fix

I don’t understand why we *were* using URL encoding for field names.  I don’t understand why we’re now inventing a custom encoding for these fields that looks by all appearances to be URL encoding, and yet cannot be decoded by the other end.
Comment 8 Alexey Proskuryakov 2009-12-10 16:23:36 PST
Mark has suggested an interesting idea for future consideration - even though RFC 2388 doesn't apply to ASCII characters, we can still use its form of encoding, e.g. =0a. But I don't think we should be changing this now.
Comment 9 Darin Adler 2009-12-10 16:35:08 PST
Comment on attachment 44644 [details]
proposed fix

This change is clearly good. Maybe we can find a better form of encoding than %-encoding in the future, but this will get rid of the regression without reintroducing the problem we originally were fixing by quoting the name.

r=me
Comment 10 Alexey Proskuryakov 2009-12-10 16:42:44 PST
Committed <http://trac.webkit.org/changeset/51973>.
Comment 11 Daniel Bates 2009-12-20 13:19:38 PST
Marking this bug as fixed since it was committed in changeset 51973 <http://trac.webkit.org/changeset/51973>. If this is a mistake then reopen this bug.
Comment 12 Alexey Proskuryakov 2009-12-20 13:32:21 PST
This was just an oversight - thanks for noticing!