Bug 9854 - HTTP Refresh header with quotes is parsed incorrectly
Summary: HTTP Refresh header with quotes is parsed incorrectly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Alexey Proskuryakov
URL: http://notmuch.com/Quiz/
Keywords:
Depends on:
Blocks:
 
Reported: 2006-07-11 10:46 PDT by Mark
Modified: 2006-12-08 10:32 PST (History)
3 users (show)

See Also:


Attachments
test case (188 bytes, text/x-perl-script)
2006-07-11 11:52 PDT, Alexey Proskuryakov
no flags Details
proposed fix (19.29 KB, patch)
2006-12-03 03:24 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark 2006-07-11 10:46:18 PDT
* 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..."
Comment 1 David Kilzer (:ddkilzer) 2006-07-11 11:00:19 PDT
(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.
Comment 2 Alexey Proskuryakov 2006-07-11 11:52:11 PDT
Created attachment 9375 [details]
test case

Ugh, looks like Refresh header parsing is quite broken, neither single nor double quotes work!
Comment 3 Mark 2006-07-15 08:21:08 PDT
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.

Comment 4 Mark 2006-07-15 14:25:42 PDT
sorry, the error message comes up after seeing the question and clicking "that's my guess", not the "answer" button...


Comment 5 Mark 2006-07-17 14:25:43 PDT
*** 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
Comment 6 Alexey Proskuryakov 2006-12-03 03:24:38 PST
Created attachment 11716 [details]
proposed fix
Comment 7 David Kilzer (:ddkilzer) 2006-12-03 04:45:16 PST
(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!

Comment 8 Alexey Proskuryakov 2006-12-03 11:15:20 PST
> 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...
Comment 9 David Kilzer (:ddkilzer) 2006-12-03 14:18:37 PST
(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.
Comment 10 Darin Adler 2006-12-07 15:15:38 PST
Comment on attachment 11716 [details]
proposed fix

+        return ok;
+    } else {

Why use else here?

r=me
Comment 11 Alexey Proskuryakov 2006-12-08 02:54:00 PST
> 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.
Comment 12 Alexey Proskuryakov 2006-12-08 10:32:30 PST
Committed revision 18077.