Bug 6345 – REGRESSION: repro crash when specifying an invalid value for the charset attribute of a <script>

• Component: WebKit Misc.
• Status: VERIFIED FIXED

Safari crashes if the charset attribute in a <script> tag, (or the document's charset, if the script tag 
doesn't specify a charset) is not a valid encoding name. The crash is because ChachedScript's m_codec is 
null.

To reproduce, open the attached testcase in Safari.

The regression is from the patch for bug 6078, which replaced a few instances of KGlobal::charsets()-
>codecForName() with QTextCodec::codecForName(). The former defaults to ISO-8859-1 when the 
argument is invalid, but the latter returns 0.

Created attachment 5438 [details]
testcase (crasher)

[Eric Seidel (no email)]

I've got a fix.

[Eric Seidel (no email)]

Created attachment 5454 [details]
The fix (with a small amount of unrelated cleanup)

[Eric Seidel (no email)]

Comment on attachment 5454 [details]
The fix (with a small amount of unrelated cleanup)

Note, this fixes 2 of the 3 places I changed from KCharsets::codecForName to
QTextCodec::codecForName.  The 3rd case (found in
khtml/html/html_formimpl.cpp), I didn't change.  The code there seems to
*expect* that codec for name might return NULL, where has
KCharset::codecForName never returned NULL.  It seems that if the first charset
listed was "invalid" we would assume the page was supposed to be latin1, now we
(hopefully correctly) go through the entire list, looking for a valid charset
first.

[Eric Seidel (no email)]

Comment on attachment 5454 [details]
The fix (with a small amount of unrelated cleanup)

The reason I didn't add a test for the accept-charset related change (which was
made by the previous bug, and is now being noticed when going to fix this
crash) is that I don't know how to go about testing local form submissions in
DRT.

[Alexey Proskuryakov]

I made a test for encodings in submitted forms which is now in fast/forms/form-data-encoding.html - 
not sure if the same approach can be used here, though.

[Darin Adler]

Comment on attachment 5454 [details]
The fix (with a small amount of unrelated cleanup)

Looks fine.

[Joost de Valk (AlthA)]

Removing keyword(s) since bug is fixed.

[Joost de Valk (AlthA)]

Removing keyword(s) since bug is fixed.

[Eric Seidel (no email)]

Removing Regression keyword from bugs already fixed.