Bug 9854 – HTTP Refresh header with quotes is parsed incorrectly

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mark]

* note: the URL above is to register to get to the quiz page for Public Radio International's "Whad'Ya Know?" Online Quiz Questions .

To get to the page where the problem exists requires registration (an email and a nickname only) then you will get to the quiz at URL:  http://notmuch.com/Tools/quiz.pl?user_id=XXXXX       (the XXXX is your unique user id - and unfortunately, there is only one question to answer per day, so I set up multiple accounts to try with multiple browsers to interact on same day...will make sense when you get there - really easy).

OK, the bug:  after answering a quiz question the page refreshes once and says your answer is received (this works in Safari / Webkit).  Then, the site is supposed to automatically refresh and go back to the quiz questions to show your answer as correct and incorrect, and your stats for the week.  This is when Safari/Webkit fail.  The error message is:

Submission Failed
Submission failed for the following reason(s):
You have an error in your SQL syntax near '"' at line 1
Query: SELECT username, username_formatted, id, email FROM users WHERE id = XXXXX"
Hit the 'back' button and try your submission again, making changes if necessary.


Firefox 1.5.0.4 and IE 5.23 both seem to navigate the site and return one to the appropriate page.  My first response was to write the webmaster and the response was  "it doesn't happen on my machine..."

[David Kilzer (:ddkilzer)]

(In reply to comment #0)
> The error message is:
> 
> Submission Failed
> Submission failed for the following reason(s):
> You have an error in your SQL syntax near '"' at line 1
> Query: SELECT username, username_formatted, id, email FROM users WHERE id =
> XXXXX"
> Hit the 'back' button and try your submission again, making changes if
> necessary.

That message is very scary from a security standpoint.  (That has nothing to do with the issue with Safari, though.)

This bug would probably benefit from some packet analysis using something like Ethereal or tcpdump.  Compare what Firefox sends to the server when answering the question with what Safari+WebKit sends.

[Alexey Proskuryakov]

Created attachment 9375 [details]
test case

Ugh, looks like Refresh header parsing is quite broken, neither single nor double quotes work!

[Mark]

seems they've made some changes on their end...or else Safari is behaving differently now to the interaction.  New error message occurs when click on "Answer Question" button--such that you can't even answer the question.  Tried to duplicate the problem in IE and FireFox and got the same response so this may be a problem on their end.  Send info to web person.

Error message after clicking "answer" question button:

Submission Failed

Submission failed for the following reason(s):

Access denied for user ''@'localhost' to database 'quiz'
Query: LOCK TABLES contest WRITE

Hit the 'back' button and try your submission again, making changes if necessary.

[Mark]

sorry, the error message comes up after seeing the question and clicking "that's my guess", not the "answer" button...

[Mark]

*** IGNORE COMMENTS 3 & 4... they switched servers this week...  

*** Now that they've got things "fixed" we are back to the original error message and situation.

Thanks all

[Alexey Proskuryakov]

Created attachment 11716 [details]
proposed fix

[David Kilzer (:ddkilzer)]

(In reply to comment #6)
> Created an attachment (id=11716) [edit]
> proposed fix

Apple's copyright is missing in new HTTPParsers.{h,cpp} files.  Nice patch!

[Alexey Proskuryakov]

> Apple's copyright is missing in new HTTPParsers.{h,cpp} files.

I was wondering whether it should be there, given that there is no Apple code in these files. Actually, it seems that Apple employees often don't preserve copyright notices even when moving old code to new files...

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> > Apple's copyright is missing in new HTTPParsers.{h,cpp} files.
> 
> I was wondering whether it should be there, given that there is no Apple code
> in these files. Actually, it seems that Apple employees often don't preserve
> copyright notices even when moving old code to new files...

After writing Comment #8, I began to wonder whether adding an Apple copyright was needed in that case or not, or whether it would only need to be added if they made future changes.

[Darin Adler]

Comment on attachment 11716 [details]
proposed fix

+        return ok;
+    } else {

Why use else here?

r=me

[Alexey Proskuryakov]

> Why use else here?

It seemed slightly more readable to me - we are handling two cases, nice to have both at the same indentation level.

[Alexey Proskuryakov]

Committed revision 18077.