[Webkit-unassigned] [Bug 20795] text/plain form encoding ignored and incorrectly specified in request header

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=20795


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #73453|review?                     |review-
               Flag|                            |




--- Comment #10 from Alexey Proskuryakov <ap at webkit.org>  2010-11-10 11:37:43 PST ---
(From update of attachment 73453)
View in context: https://bugs.webkit.org/attachment.cgi?id=73453&action=review

Please clarify what exactly this patch implements, and add test coverage. Besides what is mentioned below, there should be a test with non-ASCII data.

> WebCore/ChangeLog:13
> +        Implementation algorithm defined here:
> +        http://dev.w3.org/html5/spec/Overview.html#text-plain-encoding-algorithm

You reference the HTML5 algorithm, but is it fully implemented? We don't implement accept-charset, and we don't implement _charset_ entries. There is no test for sending a file input as text/plain.

> WebCore/loader/FormSubmission.cpp:168
> +        formData = FormData::create(*(static_cast<FormDataList*>(domFormData.get())), domFormData->encoding(), isMailtoForm || attributes.method() == GetMethod ? "application/x-www-form-urlencoded" : encodingType);

This doesn't seem to match what HTML5 says. GET/mailto is always "application/x-www-form-urlencoded", but POST/mailto uses "appropriate form encoding algorithm".

> WebCore/platform/network/FormData.h:98
> +    static PassRefPtr<FormData> create(const FormDataList&, const TextEncoding&, const String encodingType = "application/x-www-form-urlencoded");

The encoding type definitely shouldn't be passed as string copy. Maybe String reference, maybe an AtomicString, or maybe an enum constant. The constant is probably most appropriate.

> WebCore/platform/network/FormDataBuilder.cpp:190
> +        // http://dev.w3.org/html5/spec/Overview.html#text-plain-encoding-algorithm

I don't think that a link to an interim version of HTML5 is useful. That's where one would look by default anyway.

> LayoutTests/http/tests/misc/form-post-textplain.html:13
> +    <input type="hidden" name="f1" value="This is field #1 &!@$%'<>">
> +    <input type="hidden" name="f2" value='This is field #2 ""'>

Interesting characters to test should include '=' and newlines.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.