Bug 32140 - REGRESSION(r50072): Mailman administrative functionality is broken
Summary: REGRESSION(r50072): Mailman administrative functionality is broken
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.6
: P2 Normal
Assignee: Alexey Proskuryakov
URL: http://src.macosforge.org/webkit-post...
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2009-12-03 19:10 PST by William Siegrist
Modified: 2009-12-20 13:32 PST (History)
6 users (show)

See Also:


Attachments
proposed fix (4.84 KB, patch)
2009-12-10 15:53 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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!