User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2b) Gecko/20021001 Phoenix/0.2 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1) Gecko/20020826 I use a javascript function to open a new window. The method window.open(<uri>,'') replaces all special characters like one of {ä,ü,ö,ß...} by their %<hex-value> form, in the <uri> String. But all following '&', '=' are replaced too. Because of that I cannot use the uri-request-arguments to transport data from one page to another one. example: tr_id=151&mem_id=17572&mem_name=crew|&reason=Sprühen&val=13&wind_type=modify gets tr_id=151&mem_id=17572&mem_name=crew|&reason=Spr%FChen%26val%3D13%26wind_type %3Dmodify The Javascript function I used: function modTA(tr_id, memb_id, memb_name, reason, val) { var mf = window.open("clanfinance/taWindow.php?tr_id="+tr_id+"&mem_id=" +memb_id+"&mem_name="+memb_name+"&reason="+reason+"&val="+val +"&wind_type=modify", "", "height=220,width=265,resizeable=0,status=0"); } The same happens when I use the Phoenix Browser Reproducible: Always Steps to Reproduce: 1. go to the url i described above 2. navigate: -> ClanKasse -> Clankasse 3. select a row, that contains one of the characters {ö, ü, ä} like that with TrID=64 4. -> ändern ... -> use the 'Zurück' button to close the window Actual Results: the "Zweck" edit field contains the uri-request-arguments which follow the argument, which contains the data, that should be displayed in the edit field. Expected Results: Mozilla should only display the data, which the argument &reason contains. Notices: You should allow cookies to access that web page You should not modify too much on that web page

Browser, not engine ---> Form Submission for further triage. Rudolf: I get a "404 - Not Found" error when I try to go to the URL you gave above: http://thecrew.ngz-server.de/rudi/clanfinanzen_test_site/ Is there a typo?

Not Found The requested URL /rudi/clanfinanzen_test_site/ was not found on this server. Apache/1.3.26 Server at thecrew.ngz-server.de Port 80

sorry, i modified the url

I see what the problem is, but is not form submission, is the window.open method that is doing the escaping magic in the URI string. The &'s in front of the first occurence of an german umlaut character do not get escaped. This is not form submission afaict, however the form submission code does use the nsEscape function to escape occurences of chars not in the US-ASCII set. If that is the problem, it should be repaired there. I'm not familiar to the javascript part though. I will try to find out to whom to reassign this one unless Phil knows.

Alex is right; I misassigned this. The reduced testcase calls window.open() on this non-existent URL: "AAA=crew&BBB=Sprühen&CCC=modify" Mozilla raises an alert saying this URL can't be found. In the alert, we can see the bug: The 'ü' is escaped to '%FC' <--- GOOD But then things go wrong after this (Moz 20021014xx WinNT): The next '&' is escaped to '%26' <--- BAD! The next '=' is escaped to '%3D' <--- BAD! Reassigning to DOM Level 0, although perhaps they are relying on other code in order to form this escaped URL...

UPDATE: both the testcase and the original URL above WORKFORME using trunk builds from today, 2002-10-15, on WinNT and Linux. Rudolf: could you try the latest build and see if the bug is fixed? If so, you can resolve this bug as WORKSFORME. Thanks -

CORRECTION: the bug is NOT fixed. I. The reduced testcase: a) When run off the Bugzilla server, we do not see the bug. No alert comes up, and the new window opens with this message: "The requested URL /AAA=crew&BBB=Sprühen&CCC=modify was not found on this server." Since no escaping is done, we can't see the bug. b) But when saved as a local file, however, we get the bug I described in Comment #6 above. We do see this bug in the 2002-10-15 builds, too. II. The given URL: http://thecrew.ngz-server.de/rudi/clanfinanzen_test_site_moz_error/ I misunderstood what to look for in the "Zweck" textbox. If you click the "ändern" link in the "Sprühen" row, the "Zweck" textbox in the new window should contain only "Sprühen", as in IE6. Mozilla is showing "Sprühen&val=100&wind_type=modify" instead. We see this bug in the 2002-10-15 builds, too.

I can reproduce the bug also on a linux system with Netscape 7.0, Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0.

there is a difference between the way the url is encoded in the GET form submission and in window.open(). see the nsFSURLEncoded::URLEncode(nsFormSubmission.cpp) method where we use nsEscape only on the buffers that contain the names or the values but not the = and & signs. i do think bug 174615 is a dup of this one

sorry i attached a wrong testcase, will attach a corrected one

the correct testcase

*** Bug 174615 has been marked as a duplicate of this bug. ***

*** Bug 176417 has been marked as a duplicate of this bug. ***

From the duplicate bug 176417: ------- Additional Comment_ #8 From rogerl@netscape.com 2002-10-24 11:30 ------- ... The string, with the '&' intact, gets through the JS engine just fine and finally gets hosed by 'nsEscapeCount' in xpcom/io/nsEscape.cpp - called by GlobalWindowImpl::Escape.

Marking this topembed. Due to this bug, #1 chat site in China is not working. Cannot find any existing chat rooms as URL to a room include "&gender=male/female" following a Chinese character.

Marking this as topembed+/nsbeta1+ as this is effecting the top site in China.

the project under the previous url http://thecrew.ngz-server.de/... became status productive, so i created the same site at another provider, to demonstrate the bug. the new url is http://mitglied.lycos.de/meph6666/clanfinanzen_test_site/

I wanted to ask when the target milestone will be released, because I cannot find anything about that release on your actual roadmap.

*** Bug 125718 has been marked as a duplicate of this bug. ***

Another version of this bug appears for example with window.open("pageö.html?id=5") which will try to open "page%F6.html%3Fid%3D5". Neither the '?' nor the '=' should be encoded. I don't think the encoding of '?' has been mentioned previously? This breaks at least one rather major Swedish website.

Taking...

This patch adds a new bit, url_QueryChars, to nsEscapeMask. nsGlobalWindow ORs this bit into the mask parameter when it calls Escape(). If this bit is set, the three characters '?', '&', and '=' are not escaped in the url string. I've tested the whichever test cases are live on this bug and its duplicates. They all work. I'm concerned, though, that we might still be missing some cases. Darin, can you think of other characters that we shouldn't escape? How about the following example: window.open("http://savuÄlov.com:8080/return.php?name1=value1"); Should we or should we not escape the ':' in the url's host name? With the current patch, the ':' is escaped. Will that confuse Necko?

Comment on attachment 111812 [details] [diff] [review] First try at fix Heikki, would you please review this? Its a small, one page patch for the topembed+ bug that I snagged from jst last week. Johnny, if you could super-review it, that would be great!

The patch is about URL handling and I don't feel comfortable reviewing this. I think this should be reviewed by darin, andreas or dougt, or all of them. The first thought that came to me about this was that we should probably do that special escaping only for the query part of the URL, not the whole URL. But I just don't know this code well enough to know what works and what might cause regressions.

I can't believe that old nsEscape stuff is still in use ... can someone please point me to the code that uses it.

Comment on attachment 111812 [details] [diff] [review] First try at fix - In nsEscape.cpp: { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, /* 0x */ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, /* 1x */ - 0,0,0,0,0,0,0,0,0,0,7,4,0,7,7,4, /* 2x !"#$%&'()*+,-./ */ - 7,7,7,7,7,7,7,7,7,7,0,0,0,0,0,0, /* 3x 0123456789:;<=>? */ + 0,0,0,0,0,0,8,0,0,0,7,4,0,7,7,4, /* 2x !"#$%&'()*+,-./ */ + 7,7,7,7,7,7,7,7,7,7,0,0,0,8,0,8, /* 3x 0123456789:;<=>? */ 0,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7, /* 4x @ABCDEFGHIJKLMNO */ Clean up the indentation of this code while you're at it, I smell evil tabs! :-) Same thing in nsEscape.h. sr=jst

Okay, found it in nsGlobalWindow. Why does this escape happen at all? I think it is not necessary, by doing so you destroy the url syntax and then you get what you ask for. Let the escaping be done by the code that actually creates the url. I don't think the problem should be fixed with the attached patch.

If you want to escape here then you should possibly use NS_EscapeURL with the esc_OnlyNonASCII mask, but then why escape at all?

Comment on attachment 111812 [details] [diff] [review] First try at fix >Index: nsEscape.cpp > { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, /* 0x */ > 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, /* 1x */ >+ 0,0,0,0,0,0,8,0,0,0,7,4,0,7,7,4, /* 2x !"#$%&'()*+,-./ */ >+ 7,7,7,7,7,7,7,7,7,7,0,0,0,8,0,8, /* 3x 0123456789:;<=>? */ > 0,7,7,7,7,7,7,7,7,7,7,7,7,7,7,7, /* 4x @ABCDEFGHIJKLMNO */ there's some definite indentation problems here. can you please clean up the indentation along with this patch? thx! >Index: nsEscape.h > , url_Path = PR_BIT(2) >+, url_QueryChars = PR_BIT(3) again, indentation probs. r=darin

Comment on attachment 111812 [details] [diff] [review] First try at fix revoking my r= until andreas's comments are addressed (i jumped the gun). generally, the rule is to let necko do the escaping. otherwise, we lose out on knowledge of non-ASCII chars. if at possible try to leverage the escaping built into nsStandardURL.

I've broken out the string encoding code from Escape() into a new protected method called Convert() in nsGlobalWindow. Escape() now calls Convert() and then escapes the string using the old nsEscape() call, like so: nsEscape(dest.get(), nsEscapeMask(url_XAlphas | url_XPAlphas | url_Path)); OpenInternal(), however, calls Convert() and then escapes the url using NS_EscapeURL(), like so: NS_EscapeURL(dest, esc_AlwaysCopy | esc_OnlyNonASCII, rightCStr); I've also cleaned up the code in OpenInternal() that was doing a bunch of unnecessary string copies.

Comment on attachment 112354 [details] [diff] [review] Patch that incorporates comments from jst, andreas, and darin Darin and Johnny, Would you please r and sr this patch? It fixes url escaping problems in nsGlobalWindow::Open() which is a topembed+ bug right now. Thanks!

Comment on attachment 112354 [details] [diff] [review] Patch that incorporates comments from jst, andreas, and darin +GlobalWindowImpl::Convert(const nsAString& aStr, + char** aDest) Maybe this would be better named doCharsetConversion() or somesuch? - In GlobalWindowImpl::Escape(): + if (NS_SUCCEEDED(result = Convert(aStr, getter_Copies(dest)))) { Please pull this call to Convert() out before the if, and then just check NS_SUCCEEDED(result) in the if (and rename 'result' to 'rv' while you're at it). - In GlobalWindowImpl::OpenInternal(): + // if the URL contains non ASCII, escape all non ASCII characters starting + // from the first non ASCII char. I wonder if this is really worth it. We save passing some characters to the charset converter, but is it worth the additional code size and added complexity? Couldn't this just pass the whole URL to Convert() and let it deal with the fact that some of the first characters don't need converting? I'm fine with this either way, but I just don't see much of a benefit of doing what we do here. + if (NS_SUCCEEDED(Convert(Substring(aUrl, nonAsciiPos, len - nonAsciiPos), + getter_Copies(dest)))) { Indent the getter_Copies() so that it lines up with the other argument to Convert(). sr=jst

This patch includes all the changes suggested by jst. His sr can carry forward to this patch. Will ask Darin for a review so that another pair of eyes see this final (hopefully! :-) ) patch.

Comment on attachment 112466 [details] [diff] [review] Patch that addresses jst's comments Darin, here's the patch I just AIM'd you about. Please take a look. The patch is described in comment 32 and includes the changes suggestd by jst in comment 34. Thanks!

Comment on attachment 112466 [details] [diff] [review] Patch that addresses jst's comments r=darin (there's a ton of string copies in this code!)

Comment on attachment 112466 [details] [diff] [review] Patch that addresses jst's comments sr=jst

Comment on attachment 112466 [details] [diff] [review] Patch that addresses jst's comments This is a topembed+ bug that breaks China's top chat site among others. The problem was that window.open() was escaping ASCII characters like '&', '=', and '?' in the url. Now, it only escapes non-ASCII characters. darin and andreas have reviewed and jst has super-reviewed. I've tested the old test case in bug 35076 and all the test cases on this bug and its duplicates (the ones that were still accessible). I've also completed pre-checkin tests. I'm not sure how important topembed+ bugs are to the 1.3 beta release. If they are, please let me check in immediately before the 1.3 beta branch is created. If you want to get wider testing for this internally, please grant me approval to check in on the trunk after the 1.3 beta branch is created. As far as I know, Netscape project management is happy if this patch makes either 1.3 beta or 1.3 final. Thanks!

Comment on attachment 112466 [details] [diff] [review] Patch that addresses jst's comments a=asa (on behalf of drivers) for checkin to 1.3beta.

Fix checked into trunk...