Bug 7656 – Query string always appended to Flash URLs, instead of being replaced

• Component: Plug-ins
• Status: RESOLVED FIXED

[sole]

When clicking on any of the options of the menu, the url gets appended a new ?section, instead of replacing the old one. Sometimes safari gets stuck in a loop and fails with a message which says "Redirection limit exceeded". It can look like this:

Safari can’t open the page.
Too many redirects occurred trying to open “http://ifparty.scenesp.org/06/?section=news?section=about?section=press?section=register?section=news?section=press?section=contact?section=compos?section=press?section=press?section=about?section=news?section=about?section=press?section=register?section=news?section=press?section=contact?section=compos?section=press?section=press?section=about&language=sp?.... (this string is longer, fills all the browser page)

This page works properly on any other browsers we have tested: firefox mac/pc, explorer... I have been looking at the code of the page and it uses lots of redirections (302), but I haven't found any reference to safari bugs with 302 redirections. Sorry if the bug is repeated...

[Alexey Proskuryakov]

I can confirm the URL issue. It has been reported before (bug 5475), but that case was unreproducible. Setting severity to normal - blocker severity is for bugs that block WebKit development.

I didn't see the "Redirection limit exceeded" problem. Please file a separate bug for it, if it is reproducible with a current nightly build from <http://nightly.webkit.org>.

[Alexey Proskuryakov]

Created attachment 6941 [details]
proposed fix

This doesn't attempt to fix any redirection problems.

[Darin Adler]

Comment on attachment 6941 [details]
proposed fix

This patch looks very good. It needs a change log and a layout test, so I'm marking it review-. I believe we can test this with a layout test.

[Alexey Proskuryakov]

Created attachment 6945 [details]
fix with test and ChangeLog

Making a layout test was harder than fixing the bug, but not as hard as I expected :)

[Darin Adler]

Comment on attachment 6945 [details]
fix with test and ChangeLog

+            NPUTF8 urlString[size];

You should use malloc here instead of using the gcc extension that allows variable sized arrays. Maybe strdup is the best way.

But that's in test code.

Looks great, r=me.

[Alexey Proskuryakov]

Aren't variable sized arrays a standard C99 feature now?

I landed as is, although I believe that having 5 lines of code just to append a trailing zero to each string parameter doesn't look good; we'll likely want to rewrite this anyway, as the number of supported methods grows.

[Alexey Proskuryakov]

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