Bug 14774 - Submitted data only includes first input item
Summary: Submitted data only includes first input item
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: HasReduction
Depends on:
Blocks:
 
Reported: 2007-07-26 22:29 PDT by David Brown
Modified: 2008-01-02 09:35 PST (History)
3 users (show)

See Also:


Attachments
Test case from Comment #0 (248 bytes, text/html)
2007-07-27 10:01 PDT, David Kilzer (:ddkilzer)
no flags Details
Test case without password field (412 bytes, text/html)
2007-07-30 09:05 PDT, David Kilzer (:ddkilzer)
no flags Details
proposed fix (30.75 KB, patch)
2007-12-30 16:16 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff
proposed fix (32.94 KB, patch)
2008-01-01 03:01 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 David Brown 2007-07-26 22:29:22 PDT
if this form is on a web page the resulting email should have two results separated by &:
---
<form method="post" action="mailto:bogus@gmail.com">
Name: <input type="text" size="10" maxlength="40" name="name"> <br />
Password: <input type="password" size="10" maxlength="10" name="password"><br />
<input type="submit" value="Send"> 
</form>
---
Here is the result from Firefox:      name=dlb&password=photo
and from Webkit 522.12:                name=dlb
Comment 1 David Kilzer (:ddkilzer) 2007-07-27 10:00:19 PDT
Confirming the difference between Firefox 2.0.0.5 and a local debug build of WebKit r24669 with Safari 3 Public Beta v. 3.0.2 (522.12) on Mac OS X 10.4.10 (8R218).

This could be a security precaution, though, to prevent the password from being leaked.  Imagine if you walked up to a computer with the username and password filled out, and used a javascript: URL to change the action on the form to a mailto: and then submitted the form.

Comment 2 David Kilzer (:ddkilzer) 2007-07-27 10:01:10 PDT
Created attachment 15706 [details]
Test case from Comment #0

Please post test cases as attachments for easier testing in the future.  Thanks!
Comment 3 David Kilzer (:ddkilzer) 2007-07-30 09:00:06 PDT
To: David Kilzer
From: David Brown
Date: Sat, 28 Jul 2007 14:24:44 -0700
Subject:	 http://bugs.webkit.org/show_bug.cgi?id=14774

[...]

The example provided used password type field but you will find if you change the field type that the bug occurs for more than one text field as well; it's not a feature.  In any case I agree the password-type field has no significant security, but I would think that's more an issue for the WWW standards group and web developers who use it.

Sorry for improper etiquette.  This was my first bug report.  Feedback is welcome if I still have it wrong.

-- David
Comment 4 David Kilzer (:ddkilzer) 2007-07-30 09:05:00 PDT
Created attachment 15744 [details]
Test case without password field

This test case without a password field fails as well.
Comment 5 David Kilzer (:ddkilzer) 2007-07-30 09:06:38 PDT
(In reply to comment #1)
> Confirming the difference between Firefox 2.0.0.5 and a local debug build of
> WebKit r24669 with Safari 3 Public Beta v. 3.0.2 (522.12) on Mac OS X 10.4.10
> (8R218).

Note that Safari 2.0.4 (419.3) on Mac OS X 10.4.10 (8R218) behaves the same way, so this is not a regression.

Comment 6 Alexey Proskuryakov 2007-12-30 16:16:59 PST
Created attachment 18198 [details]
proposed fix

Fixes this and many other issues with submitting mailto forms.

Tested GET with Firefox and IE, but only tested POST with Firefox, as IE immediately sends such e-mails instead of opening them in a mail client.

A reconversion in text/plain case looks pretty ugly, I would appreciate suggestions on how to write it in a better way.
Comment 7 Darin Adler 2007-12-30 22:35:02 PST
Comment on attachment 18198 [details]
proposed fix

Will this work if you submit the same form multiple times?

mailtoForm is not a great name for a boolean; it sounds like it's a form. Maybe isMailtoForm would be a better name? I guess it's not your name.

     bool mailtoForm = u.protocol() == "mailto";

This one is case sensitive, but the new code is correctly folding case.

I'm not happy that this code still has to use DeprecatedString even in this line of code:

+                data = new FormData((DeprecatedString("body=") + encodeCString(body.latin1())).replace('+', "%20").latin1());

Isn't there a way to do this without DeprecatedString and without widening and re-narrowing?

r=me
Comment 8 Alexey Proskuryakov 2007-12-31 00:22:41 PST
(In reply to comment #7)
> Will this work if you submit the same form multiple times?

  Not sure how to test for this. Is calling form.submit() twice the test for it?

> mailtoForm is not a great name for a boolean; it sounds like it's a form. Maybe
> isMailtoForm would be a better name?

  Will do.

>      bool mailtoForm = u.protocol() == "mailto";
> This one is case sensitive, but the new code is correctly folding case.

  Will do.


> I'm not happy that this code still has to use DeprecatedString even in this
> line of code:
> 
> +                data = new FormData((DeprecatedString("body=") +
> encodeCString(body.latin1())).replace('+', "%20").latin1());
> 
> Isn't there a way to do this without DeprecatedString and without widening and
> re-narrowing?

  Yes, I can get rid of DeprecatedString here. Not sure what you mean by narrowing and re-narrowing (though as I mentioned in a comment, the hack for text/plain looks quite ugly, and I'd appreciate ideas about how to write this in a abetter way).
Comment 9 Alexey Proskuryakov 2008-01-01 03:01:47 PST
Created attachment 18224 [details]
proposed fix

Addressed review comments that I could address (see questions above).
Comment 10 Darin Adler 2008-01-01 09:51:03 PST
Comment on attachment 18224 [details]
proposed fix

r=me
Comment 11 Alexey Proskuryakov 2008-01-02 09:35:32 PST
Committed revision 29086.