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

VERIFIED FIXED in mozilla1.3alpha

Status

()

Core
HTML: Form Submission
P1
normal
VERIFIED FIXED
15 years ago
15 years ago

People

(Reporter: Sam Mefford, Assigned: bz)

Tracking

({compat})

Trunk
mozilla1.3alpha
compat
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

15 years ago
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.
to jkeiser.
Assignee: alexsavulov → jkeiser
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Keywords: compat
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....
OS: Windows 2000 → All
Hardware: PC → All
Created attachment 101325 [details] [diff] [review]
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!

Updated

15 years ago
Priority: -- → P4
(Reporter)

Comment 5

15 years ago
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.
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.
Assignee: jkeiser → bzbarsky
Severity: major → critical
Priority: P4 → P1
Target Milestone: --- → mozilla1.2beta
Created attachment 103026 [details]
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.
Created attachment 103027 [details]
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.
So I'm not sure what the state of the submission location is, but the security
check still seems wrong.
Here goes one last-ditch attempt to get a security person to comment on this
before I just sorta decide something....
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.
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...
Severity: critical → normal
Priority: P1 → P2
Target Milestone: mozilla1.2beta → mozilla1.3alpha
Comment on attachment 101325 [details] [diff] [review]
patch, untested

r=jkeiser
Submitting to the document is the right behavior.
Attachment #101325 - Flags: review+
Depends on: 177237
(Reporter)

Comment 14

15 years ago
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?
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.
Created attachment 105160 [details] [diff] [review]
patch, tested and all
Attachment #101325 - Attachment is obsolete: true
jkeiser?  review?
Priority: P2 → P1
Summary: Bad change in <form> (with no action) behavior → [FIX]Bad change in <form> (with no action) behavior
Comment on attachment 105160 [details] [diff] [review]
patch, tested and all

r=jkeiser
Attachment #105160 - Flags: review+
Attachment #105160 - Flags: superreview+
Comment on attachment 105160 [details] [diff] [review]
patch, tested and all

sr=peterv
checked in for 1.3a
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 21

15 years ago
verifying build 2003-02-12-08-trunk
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.