171924 - [FIX]Bad change in <form> (with no action) behavior

Status: VERIFIED FIXED

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

Description

[Sam Mefford]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020827
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020827

The <form> tag no longer behaves properly when no action is specified and the
page has a base href.  Perhaps according to someone's interpretation of the
spec, this is an improvement, but my code depends on the behavior existant in
IE, and Netscape < 7

The behavior I'm used to is that a <form> with no action submits to itself,
regardless of the existance of a <base href> tag.  But the new version of
Netscape (and mozilla 1.0+) submits to the base href instead of the original
document.

I didn't worry about it when I saw it in Mozilla since I primarilly use Linux
and Mozilla and I'm used to bugs, but now that Netscape 7.0 is released, I'm
getting reports of bugs from end users.

Another extra-weird thing about this is it works fine and as expected if the
base href does not begin with http://.

Reproducible: Always

Steps to Reproduce:
1. Create a page like http://reliv.stage.icentris.com/netscapeBug.htm
2. View it, submit the form
3. If you go to 

  http://reliv.stage.icentris.com/netscapeBug.htm?testname=testvalue 

all is well, if you go to 

  http://reliv.stage.icentris.com/help/wrong/place?testname=testvalue

you've got a problem.

Actual Results:  
In Mozilla < 1 and Netscape <= 6.2, I go to 

  http://reliv.stage.icentris.com/netscapeBug.htm?testname=testvalue

In Mozilla 1+ and Netscape 7.0, I go to

  http://reliv.stage.icentris.com/help/wrong/place?testname=testvalue

Expected Results:  
When there's no action, submit form to the document rather than the base href.

Comment 1

[Boris Zbarsky [:bzbarsky]]

to jkeiser.

Comment 2

[Christopher Hoess (gone)]

This is probably worth fixing, but relying on a <form> with no action="..." to
*work*, regardless of what URL it submits to, is stupendously dangerous.

Comment 3

[Boris Zbarsky [:bzbarsky]]

The code in question is at
http://lxr.mozilla.org/seamonkey/source/content/html/content/src/nsHTMLFormElement.cpp#1058
-- we check for an empty url and "reload" the document, but we need to be using
the document's uri, not the base uri....

Sam, it's really a good idea to report bugs when you see them... this fix will
obviously not be making 7.0 and may not even make the next NS release....

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: patch, untested

I can't build at the moment, so totally untested... I made the diff big enough
to highlight the real issue here -- the call to securityManager->CheckLoadURI
was using the baseURL instead of the docURL as well!

Comment 5

[Sam Mefford]

The priority was just set to P4.  Is it inappropriate for me to request it be a
little higher priority?

This bug breaks compatibility with IE, NS4, and previous versions of Mozilla.

I have to disagree with Comment #2 From Christopher Hoess.  I won't discuss the
full detail here, but I can give many reasons the <form> with not action="..."
is very important.  Because enough other browsers submit the user back where
they were, many web applications rely on this behavior.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Heh,  Well, since I'm the guy with the patch, I get to set the priority... ;) 
This totally slipped my mind.

Mitch, could you please look at this patch?  The use of the <base> for the
security check _seems_ wrong, but are we doing that for a good reason?  Note
that nsScriptLoader.cpp also uses the base URI for the security check in
nsScriptLoader::ProcessScriptElement.  That also seems very wrong to me...

Raising severity to correspond to my evaluation of this as a security issue, btw.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase like the reporter's

This testcase, and http://reliv.stage.icentris.com/netscapeBug.htm, work fine
for me.  The same page is reloaded.  This is likely because we no longer
process such <base> tags.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Modified testcase

This testcase is modified to put the <base> in the <head>.  NS4 and Mozilla
both go to the base url in this one; I have no IE on hand to test.

Comment 9

[Boris Zbarsky [:bzbarsky]]

So I'm not sure what the state of the submission location is, but the security
check still seems wrong.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Here goes one last-ditch attempt to get a security person to comment on this
before I just sorta decide something....

Comment 11

[Daniel Veditz [:dveditz]]

I can confirm that with the modified testcase both N4.79 and Mozilla go to the
specified <base> URL while IE submits to the actual page. In the URL given by
the reporter current trunk builds and 4.x go to the given page, while 1.0-ish
builds go to the wrong page.

Since the <BASE> behavior change brought us back into compatibility with
historical Navigator (4.x) behavior--although still different from IE--this bug
could be closed WFM.

I agree w/bz, though, that calling CheckLoadURI() with the base URI--which could
be anything--is not a good security check. Might as well not have the check in
that case since any potential attacker could just set the base to be equal to
the intended target.

Comment 12

[Boris Zbarsky [:bzbarsky]]

OK.  I have done testing in IE now.  In IE, the "testcase like the reporter's"
and "modified testcase" both go to the same page.  This is also the page that
NS4 and current Mozilla go to in "testcase like the reporter's".  In "modified
testcase" NS4 and Mozilla go to the same page as each other, but not the same as IE.

I'm not sure what we want to do here.  In any case, the security check needs to
be fixed, so I filed bug 177237 on it.  Reducing severity of this accordingly. 
Since we're now compatible with NS4, I would be tempted to mark this worksforme...

Comment 13

[John Keiser (jkeiser)]

Comment on attachment 101325 [details] [diff] [review]
patch, untested

r=jkeiser
Submitting to the document is the right behavior.

Comment 14

[Sam Mefford]

Sorry to be such a vocal "outsider", but I have to reiterate: YOU DO NOT HAVE
COMPATIBILITY WITH NAVIGATOR 4.x.  

I wish there were no "Modified testcase" attached to this bug.  The original
testcase shows that as long as the <form> is before the <base>, Netscape 4.7 and
IE behave compatibly (and properly, IMHO).  The "Modified testcase" shows the NS
4 _BUG_ that has been duplicated in Mozilla 1+.  

I'm not involved in the Mozilla code, nor am I an application programmer, but I
am a programmer--wouldn't it be easier to just fix this than argue about it?

Please, I'm begging, don't sideline this issue.  I have been waiting for years
to say Netscape is better than IE again, and I had just started saying it again
when I found this bug in public releases.  Right now my web applications are
broken on Netscape 7.  They work on NS 4.x, they work on IE.

Can we please put this back on track for mozilla1.2apha?

Comment 15

[Boris Zbarsky [:bzbarsky]]

Sam, as I repeatedly stated, on the first testcase IE, NS4, and current Mozilla
(but _not_ 1.1 which had a bug in <base> handling in general) all behave the same.

We all seem to agree on the second point that on the second testcase it would be
better to go with the IE behavior.

If it were up to me and the tree were _open_ this would be checked in by now. 
But the tree is closed.  Which means I need approval.  Which means I need to
resolve the security issue.  Which means this is blocked by bug 177237.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Attachment: patch, tested and all

Comment 17

[Boris Zbarsky [:bzbarsky]]

jkeiser?  review?

Comment 18

[John Keiser (jkeiser)]

Comment on attachment 105160 [details] [diff] [review]
patch, tested and all

r=jkeiser

Comment 19

[Peter Van der Beken [:peterv]]

Comment on attachment 105160 [details] [diff] [review]
patch, tested and all

sr=peterv

Comment 20

[Boris Zbarsky [:bzbarsky]]

checked in for 1.3a

Comment 21

[Rakesh Mishra]

verifying build 2003-02-12-08-trunk