23376 - Form action of "foo?bar=baz" method=get results in two ?'s

Status: VERIFIED FIXED

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

Description

[Eric Pollmann]

Mike Shaver wrote:

 [e-mail] wrote:
 >  <form action="crappy.cgi?some=thing" method=GET>
 >   <input type=hidden name="var1" value="val1">
 >   <input type=hidden name="var2" value="val2">
 >   <input type=submit value=" EDIT ">
 >  </form>
 > ...
 >
 > Netscape and IE both seem to realize that there's already parameters included
 > when constructing the GET URL.  So they use:
 > http://server/status/crappy.cgi?some=thing&var1=val1&var2=val2
 >
 > Mozilla just pastes everything together, sending:
 > http://server/status/crappy.cgi?some=thing?var1=val1&var2=val2

That sounds like a bug to me.  Neckoites?  Eric?

Mike

Comment 1

[Eric Pollmann]

Quick guess at a fix:

Index: nsFormFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsFormFrame.cpp,v
retrieving revision 3.88
diff -c -r3.88 nsFormFrame.cpp
*** nsFormFrame.cpp     2000/01/06 22:07:21     3.88
--- nsFormFrame.cpp     2000/01/07 22:26:33
***************
*** 627,641 ****
      GetTarget(&target);

      if (!isPost) {
!       if (href.Last() == '?') {
!         // Already specifies a '?' in the href so don't add another one
!         href.Append(data);
!       }
!       else {
           // Doesn't have a ? in the href so add one
          href.Append('?');
-         href.Append(data);
        }
      }
      nsAutoString absURLSpec;
      result = NS_MakeAbsoluteURI(href, docURL, absURLSpec);
--- 631,641 ----
      GetTarget(&target);

      if (!isPost) {
!       if (href.FindChar('?', PR_FALSE, 0) == kNotFound) {
          // Doesn't have a ? in the href so add one
          href.Append('?');
        }
+       href.Append(data);
      }
      nsAutoString absURLSpec;
      result = NS_MakeAbsoluteURI(href, docURL, absURLSpec);

Comment 2

[Eric Pollmann]

Testing of IE 5.01, Navigator 4.7 and Opera 3.61 revealed this:

               Action                Submitted
IE and Nav   foo.cgi?bar=baz      foo.cgi?var1=val1&var2=val2
             foo.cgi?bar=baz&     foo.cgi?var1=val1&var2=val2
Opera        foo.cgi?bar=baz      foo.cgi?bar=baz&var1=val1&var2=val2
             foo.cgi?bar=baz&     foo.cgi?bar=baz&&var1=val1&var2=val2

I'll look in the spec and see if there is any hint which is the right
behaviour.  (Just notice that my above fix will not add a & if needed to
separate existing variables from ones that will be added, easy to fix...)

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

We probably want to insert an ampersand before "data", no?

4.x just loses whatever data is in the ACTION's query string:

<form action="foo.html?a=b&">
<input name="c" value="d">
<input type=submit>
</form>

That will go to: "foo.html?c=d".

Comment 4

[Eric Pollmann]

The HTML4 spec says: (17.13.1)
  get: With the HTTP "get" method, the form data set is appended to the URI
    specified by the action attribute (with a question-mark ("?") as separator)
    and this new URI is sent to the processing agent.

The HTTP1.1 spec defines this as a valid form for the URI: (3.2.2)
  http_URL = "http:" "//" host [ ":" port ] [ abs_path [ "?" query ]]

Thus we should take the valid URI: "foo.cgi?bar=baz"
and append the query string "var1=val1&var2=val2" to it.

The question is, should we leave the current query string "bar=baz" specified in
the action attribute of the form, or should we axe it as IE and Nav do?  I'm
going to take exception with IE and Nav here and notice that the operative word
in the HTML4 spec is "append".  Append operations are always additive.  Thus, it
makes more sense to do as Opera does and add the two query strings together.
However, it also makes sense not add the extra & as Opera does in the second
example.

It also seems extremely reasonable for a web designer who specifies a URL
containing a query string to expect that the variables in the query string will
be sent to the server.  Why else include them?  I'll attach an updated proposed
fix...

Comment 5

[Eric Pollmann]

Attachment: Proposed fix, adds ? and & iff needed.

Comment 6

[Eric Pollmann]

Attachment: Test case

Comment 7

[Eric Pollmann]

This works for me.  Anyone want to review this so I can check it in to M13?  :)

Comment 8

[Eric Pollmann]

Just checked in this fix.

To verify, view the attachment "Test case".  Clicking to submit the third and
fourth forms should no longer result in two '?'s in the location bar.  If so,
the bug is fixed.

Comment 9

[Eric Pollmann]

See bug 25330.  I'll change the "append" operation to be like Nav and IE's.  I
guess some pages depend on this.  :)

Comment 10

[Vladimir Ermakov]

Verifying on 
 -Windows 98 build 2000-09-22-08-M18 
 -Linux RedHat6.2 build 2000-09-19-21-M18