From 00997930358c330f0680d7e2c98cfe4188f9e1b6 Mon Sep 17 00:00:00 2001 From: "L. David Baron" Date: Tue, 3 May 2011 13:19:18 -0700 Subject: [PATCH 1/6] Pass whether we're in a string to ParseAndAppendEscape. (Bug 384672, patch 1) r=bzbarsky This passes true for string tokens and for url() tokens containing string, since that is what CSS 2.1 chapter 4's tokenization suggests. --- layout/style/nsCSSScanner.cpp | 10 +++++----- layout/style/nsCSSScanner.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/layout/style/nsCSSScanner.cpp b/layout/style/nsCSSScanner.cpp index 9be202e130f..6c85b14a5cd 100644 --- a/layout/style/nsCSSScanner.cpp +++ b/layout/style/nsCSSScanner.cpp @@ -971,7 +971,7 @@ nsCSSScanner::NextURL(nsCSSToken& aToken) ch = Read(); if (ch < 0) break; if (ch == CSS_ESCAPE) { - ParseAndAppendEscape(ident); + ParseAndAppendEscape(ident, PR_FALSE); } else if (IsWhitespace(ch)) { // Whitespace is allowed at the end of the URL EatWhiteSpace(); @@ -1003,7 +1003,7 @@ nsCSSScanner::NextURL(nsCSSToken& aToken) void -nsCSSScanner::ParseAndAppendEscape(nsString& aOutput) +nsCSSScanner::ParseAndAppendEscape(nsString& aOutput, PRBool aInString) { PRInt32 ch = Peek(); if (ch < 0) { @@ -1076,7 +1076,7 @@ PRBool nsCSSScanner::GatherIdent(PRInt32 aChar, nsString& aIdent) { if (aChar == CSS_ESCAPE) { - ParseAndAppendEscape(aIdent); + ParseAndAppendEscape(aIdent, PR_FALSE); } else if (0 < aChar) { aIdent.Append(aChar); @@ -1103,7 +1103,7 @@ nsCSSScanner::GatherIdent(PRInt32 aChar, nsString& aIdent) aChar = Read(); if (aChar < 0) break; if (aChar == CSS_ESCAPE) { - ParseAndAppendEscape(aIdent); + ParseAndAppendEscape(aIdent, PR_FALSE); } else if (IsIdent(aChar)) { aIdent.Append(PRUnichar(aChar)); } else { @@ -1367,7 +1367,7 @@ nsCSSScanner::ParseString(PRInt32 aStop, nsCSSToken& aToken) break; } if (ch == CSS_ESCAPE) { - ParseAndAppendEscape(aToken.mIdent); + ParseAndAppendEscape(aToken.mIdent, PR_TRUE); } else { aToken.mIdent.Append(ch); } diff --git a/layout/style/nsCSSScanner.h b/layout/style/nsCSSScanner.h index 882458ffd1d..9f3b38a49f0 100644 --- a/layout/style/nsCSSScanner.h +++ b/layout/style/nsCSSScanner.h @@ -215,7 +215,7 @@ protected: PRBool LookAheadOrEOF(PRUnichar aChar); // expect either aChar or EOF void EatWhiteSpace(); - void ParseAndAppendEscape(nsString& aOutput); + void ParseAndAppendEscape(nsString& aOutput, PRBool aInString); PRBool ParseIdent(PRInt32 aChar, nsCSSToken& aResult); PRBool ParseAtKeyword(PRInt32 aChar, nsCSSToken& aResult); PRBool ParseNumber(PRInt32 aChar, nsCSSToken& aResult); From 13f8891b62a821e316c320a7c5c325277f4c0707 Mon Sep 17 00:00:00 2001 From: "L. David Baron" Date: Tue, 3 May 2011 13:19:19 -0700 Subject: [PATCH 2/6] Handle failure of GatherIdent, which can (starting with the next patch) fail when the character sequence is not an identifier. Additionally, change the tokenization of a hash (#) followed by a non-name character or EOF to tokenize as DELIM (eCSSToken_Symbol) rather than as eCSSToken_Hash. This only changes the behavior in the EOF case, because the only caller (color parsing) that accepts eCSSToken_Hash (rather than only eCSSToken_ID) checks the length. (Bug 384672, patch 2) r=bzbarsky --- layout/style/nsCSSScanner.cpp | 36 +++++++++++++++++++-------- layout/style/test/test_selectors.html | 24 ++++++++++++++++++ 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/layout/style/nsCSSScanner.cpp b/layout/style/nsCSSScanner.cpp index 6c85b14a5cd..f4915789142 100644 --- a/layout/style/nsCSSScanner.cpp +++ b/layout/style/nsCSSScanner.cpp @@ -1071,6 +1071,10 @@ nsCSSScanner::ParseAndAppendEscape(nsString& aOutput, PRBool aInString) * will be aIdent with all of the identifier characters appended * until the first non-identifier character is seen. The termination * character is unread for the future re-reading. + * + * Returns failure when the character sequence does not form an ident at + * all, in which case the caller is responsible for pushing back or + * otherwise handling aChar. (This occurs only when aChar is '\'.) */ PRBool nsCSSScanner::GatherIdent(PRInt32 aChar, nsString& aIdent) @@ -1117,19 +1121,24 @@ nsCSSScanner::GatherIdent(PRInt32 aChar, nsString& aIdent) PRBool nsCSSScanner::ParseRef(PRInt32 aChar, nsCSSToken& aToken) { - aToken.mIdent.SetLength(0); - aToken.mType = eCSSToken_Ref; + // Fall back for when we don't have name characters following: + aToken.mType = eCSSToken_Symbol; + aToken.mSymbol = aChar; + PRInt32 ch = Read(); if (ch < 0) { - return PR_FALSE; + return PR_TRUE; } if (IsIdent(ch) || ch == CSS_ESCAPE) { // First char after the '#' is a valid ident char (or an escape), // so it makes sense to keep going - if (StartsIdent(ch, Peek())) { - aToken.mType = eCSSToken_ID; + nsCSSTokenType type = + StartsIdent(ch, Peek()) ? eCSSToken_ID : eCSSToken_Ref; + aToken.mIdent.SetLength(0); + if (GatherIdent(ch, aToken.mIdent)) { + aToken.mType = type; + return PR_TRUE; } - return GatherIdent(ch, aToken.mIdent); } // No ident chars after the '#'. Just unread |ch| and get out of here. @@ -1143,7 +1152,9 @@ nsCSSScanner::ParseIdent(PRInt32 aChar, nsCSSToken& aToken) nsString& ident = aToken.mIdent; ident.SetLength(0); if (!GatherIdent(aChar, ident)) { - return PR_FALSE; + aToken.mType = eCSSToken_Symbol; + aToken.mSymbol = aChar; + return PR_TRUE; } nsCSSTokenType tokenType = eCSSToken_Ident; @@ -1167,7 +1178,11 @@ nsCSSScanner::ParseAtKeyword(PRInt32 aChar, nsCSSToken& aToken) { aToken.mIdent.SetLength(0); aToken.mType = eCSSToken_AtKeyword; - return GatherIdent(0, aToken.mIdent); + if (!GatherIdent(0, aToken.mIdent)) { + aToken.mType = eCSSToken_Symbol; + aToken.mSymbol = PRUnichar('@'); + } + return PR_TRUE; } PRBool @@ -1287,10 +1302,9 @@ nsCSSScanner::ParseNumber(PRInt32 c, nsCSSToken& aToken) // Look at character that terminated the number if (c >= 0) { if (StartsIdent(c, Peek())) { - if (!GatherIdent(c, ident)) { - return PR_FALSE; + if (GatherIdent(c, ident)) { + type = eCSSToken_Dimension; } - type = eCSSToken_Dimension; } else if ('%' == c) { type = eCSSToken_Percentage; value = value / 100.0f; diff --git a/layout/style/test/test_selectors.html b/layout/style/test/test_selectors.html index 44ea328849b..c742059fc3b 100644 --- a/layout/style/test/test_selectors.html +++ b/layout/style/test/test_selectors.html @@ -233,6 +233,18 @@ function run() { }); } + function test_unparseable_via_api(selector) + { + try { + // Test that it is also unparseable when followed by EOF. + ifdoc.body.mozMatchesSelector(selector); + ok(false, "selector '" + selector + "' plus EOF is parse error"); + } catch(ex) { + is(ex.code, DOMException.SYNTAX_ERR, + "selector '" + selector + "' plus EOF is parse error"); + } + } + function test_balanced_unparseable(selector) { var zi1 = ++gCounter; @@ -248,6 +260,7 @@ function run() { "selector " + selector + " error was recovered from"); ifdoc.body.innerHTML = ""; style_text.data = ""; + test_unparseable_via_api(selector); } function test_unbalanced_unparseable(selector) @@ -263,6 +276,7 @@ function run() { "sheet should have no rules since " + selector + " is parse error"); ifdoc.body.innerHTML = ""; style_text.data = ""; + test_unparseable_via_api(selector); } @@ -891,6 +905,16 @@ function run() { bodychildset([2, 3, 8]), bodychildset([0, 1, 4, 5, 6, 7])); + // Test that we don't tokenize an empty HASH. + test_balanced_unparseable("#"); + test_balanced_unparseable("# "); + test_balanced_unparseable("#, p"); + test_balanced_unparseable("# , p"); + test_balanced_unparseable("p #"); + test_balanced_unparseable("p # "); + test_balanced_unparseable("p #, p"); + test_balanced_unparseable("p # , p"); + run_deferred_tests(); } From 2887557e927a631c27253313fce1ed570ea8605e Mon Sep 17 00:00:00 2001 From: "L. David Baron" Date: Tue, 3 May 2011 13:19:19 -0700 Subject: [PATCH 3/6] Allow ParseAndAppendEscape to fail when the stream does not contain an escape, and make callers handle this failure appropriately. This changes our behavior when backslash immediately precedes end-of-stream. (Bug 384672, patch 3) r=bzbarsky --- layout/style/nsCSSScanner.cpp | 44 ++++++++++++++++++++++----- layout/style/nsCSSScanner.h | 4 +-- layout/style/test/test_selectors.html | 3 ++ 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/layout/style/nsCSSScanner.cpp b/layout/style/nsCSSScanner.cpp index f4915789142..beb88a70904 100644 --- a/layout/style/nsCSSScanner.cpp +++ b/layout/style/nsCSSScanner.cpp @@ -971,7 +971,11 @@ nsCSSScanner::NextURL(nsCSSToken& aToken) ch = Read(); if (ch < 0) break; if (ch == CSS_ESCAPE) { - ParseAndAppendEscape(ident, PR_FALSE); + if (!ParseAndAppendEscape(ident, PR_FALSE)) { + ok = PR_FALSE; + Pushback(ch); + break; + } } else if (IsWhitespace(ch)) { // Whitespace is allowed at the end of the URL EatWhiteSpace(); @@ -1002,13 +1006,16 @@ nsCSSScanner::NextURL(nsCSSToken& aToken) } -void +/** + * Returns whether an escape was succesfully parsed; if it was not, + * the backslash needs to be its own symbol token. + */ +PRBool nsCSSScanner::ParseAndAppendEscape(nsString& aOutput, PRBool aInString) { PRInt32 ch = Peek(); if (ch < 0) { - aOutput.Append(CSS_ESCAPE); - return; + return PR_FALSE; } if (IsHexDigit(ch)) { PRInt32 rv = 0; @@ -1054,7 +1061,7 @@ nsCSSScanner::ParseAndAppendEscape(nsString& aOutput, PRBool aInString) if (IsWhitespace(ch)) Pushback(ch); } - return; + return PR_TRUE; } // "Any character except a hexidecimal digit can be escaped to // remove its special meaning by putting a backslash in front" @@ -1063,6 +1070,8 @@ nsCSSScanner::ParseAndAppendEscape(nsString& aOutput, PRBool aInString) if ((ch > 0) && (ch != '\n')) { aOutput.Append(ch); } + + return PR_TRUE; } /** @@ -1080,7 +1089,9 @@ PRBool nsCSSScanner::GatherIdent(PRInt32 aChar, nsString& aIdent) { if (aChar == CSS_ESCAPE) { - ParseAndAppendEscape(aIdent, PR_FALSE); + if (!ParseAndAppendEscape(aIdent, PR_FALSE)) { + return PR_FALSE; + } } else if (0 < aChar) { aIdent.Append(aChar); @@ -1107,7 +1118,10 @@ nsCSSScanner::GatherIdent(PRInt32 aChar, nsString& aIdent) aChar = Read(); if (aChar < 0) break; if (aChar == CSS_ESCAPE) { - ParseAndAppendEscape(aIdent, PR_FALSE); + if (!ParseAndAppendEscape(aIdent, PR_FALSE)) { + Pushback(aChar); + break; + } } else if (IsIdent(aChar)) { aIdent.Append(PRUnichar(aChar)); } else { @@ -1381,7 +1395,21 @@ nsCSSScanner::ParseString(PRInt32 aStop, nsCSSToken& aToken) break; } if (ch == CSS_ESCAPE) { - ParseAndAppendEscape(aToken.mIdent, PR_TRUE); + if (!ParseAndAppendEscape(aToken.mIdent, PR_TRUE)) { + aToken.mType = eCSSToken_Bad_String; + Pushback(ch); +#ifdef CSS_REPORT_PARSE_ERRORS + // For strings, the only case where ParseAndAppendEscape will + // return false is when there's a backslash to start an escape + // immediately followed by end-of-stream. In that case, the + // correct tokenization is badstring *followed* by a DELIM for + // the backslash, but as far as the author is concerned, it + // works pretty much the same as an unterminated string, so we + // use the same error message. + ReportUnexpectedToken(aToken, "SEUnterminatedString"); +#endif + break; + } } else { aToken.mIdent.Append(ch); } diff --git a/layout/style/nsCSSScanner.h b/layout/style/nsCSSScanner.h index 9f3b38a49f0..ed82e3dc703 100644 --- a/layout/style/nsCSSScanner.h +++ b/layout/style/nsCSSScanner.h @@ -214,8 +214,8 @@ protected: PRBool LookAhead(PRUnichar aChar); PRBool LookAheadOrEOF(PRUnichar aChar); // expect either aChar or EOF void EatWhiteSpace(); - - void ParseAndAppendEscape(nsString& aOutput, PRBool aInString); + + PRBool ParseAndAppendEscape(nsString& aOutput, PRBool aInString); PRBool ParseIdent(PRInt32 aChar, nsCSSToken& aResult); PRBool ParseAtKeyword(PRInt32 aChar, nsCSSToken& aResult); PRBool ParseNumber(PRInt32 aChar, nsCSSToken& aResult); diff --git a/layout/style/test/test_selectors.html b/layout/style/test/test_selectors.html index c742059fc3b..37d7dc6ac35 100644 --- a/layout/style/test/test_selectors.html +++ b/layout/style/test/test_selectors.html @@ -915,6 +915,9 @@ function run() { test_balanced_unparseable("p #, p"); test_balanced_unparseable("p # , p"); + // Test that a backslash alone is not treated as an escape. + test_unparseable_via_api("#\\"); + run_deferred_tests(); } From 8b0a4909bcdaafb1de71035de15c7dc317331154 Mon Sep 17 00:00:00 2001 From: "L. David Baron" Date: Tue, 3 May 2011 13:19:19 -0700 Subject: [PATCH 4/6] Only allow escaped newlines inside strings (which includes url() tokens that contain strings). (Bug 384672, patch 4) r=bzbarsky --- layout/style/nsCSSScanner.cpp | 16 +++++++++++++--- layout/style/test/test_parse_rule.html | 3 +-- layout/style/test/test_parse_url.html | 7 +++++++ layout/style/test/test_selectors.html | 7 +++++++ 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/layout/style/nsCSSScanner.cpp b/layout/style/nsCSSScanner.cpp index beb88a70904..5124967e571 100644 --- a/layout/style/nsCSSScanner.cpp +++ b/layout/style/nsCSSScanner.cpp @@ -1013,13 +1013,14 @@ nsCSSScanner::NextURL(nsCSSToken& aToken) PRBool nsCSSScanner::ParseAndAppendEscape(nsString& aOutput, PRBool aInString) { - PRInt32 ch = Peek(); + PRInt32 ch = Read(); if (ch < 0) { return PR_FALSE; } if (IsHexDigit(ch)) { PRInt32 rv = 0; int i; + Pushback(ch); for (i = 0; i < 6; i++) { // up to six digits ch = Read(); if (ch < 0) { @@ -1066,8 +1067,17 @@ nsCSSScanner::ParseAndAppendEscape(nsString& aOutput, PRBool aInString) // "Any character except a hexidecimal digit can be escaped to // remove its special meaning by putting a backslash in front" // -- CSS1 spec section 7.1 - ch = Read(); // Consume the escaped character - if ((ch > 0) && (ch != '\n')) { + if (ch == '\n') { + if (!aInString) { + // Outside of strings (which includes url() that contains a + // string), escaped newlines aren't special, and just tokenize as + // eCSSToken_Symbol (DELIM). + Pushback(ch); + return PR_FALSE; + } + // In strings (and in url() containing a string), escaped newlines + // are just dropped to allow splitting over multiple lines. + } else { aOutput.Append(ch); } diff --git a/layout/style/test/test_parse_rule.html b/layout/style/test/test_parse_rule.html index 446c7f6aff6..78cd8c36c9e 100644 --- a/layout/style/test/test_parse_rule.html +++ b/layout/style/test/test_parse_rule.html @@ -131,8 +131,7 @@ base + "#a {color: rgb(255.0, 0, 0)}", "#a {color: rgb(0%, 49.999999999999%, 0%)}", ], prop: "color", pseudo: "", todo: { - "div[title~='weeqweqeweasd\\D\\A a']{color:green}" : 1, - "div {color:green}#a\\\n{color:red}" : 1 + "div[title~='weeqweqeweasd\\D\\A a']{color:green}" : 1 } }, diff --git a/layout/style/test/test_parse_url.html b/layout/style/test/test_parse_url.html index e9b53bcdc7a..ab1b9c4d2f7 100644 --- a/layout/style/test/test_parse_url.html +++ b/layout/style/test/test_parse_url.html @@ -180,6 +180,13 @@ div.setAttribute("style", "color: url(/*); color: green"); is(div.style.color, 'green', "URL tokenized correctly outside properties taking URLs"); +div.style.listStyleImage = 'url("foo\\\nbar1")'; +is(div.style.listStyleImage, 'url("foobar1")', + "escaped newline allowed in string form of URL"); +div.style.listStyleImage = 'url(foo\\\nbar2)'; +is(div.style.listStyleImage, 'url("foobar1")', + "escaped newline NOT allowed in NON-string form of URL"); + diff --git a/layout/style/test/test_selectors.html b/layout/style/test/test_selectors.html index 37d7dc6ac35..7eeadb719fd 100644 --- a/layout/style/test/test_selectors.html +++ b/layout/style/test/test_selectors.html @@ -918,6 +918,13 @@ function run() { // Test that a backslash alone is not treated as an escape. test_unparseable_via_api("#\\"); + // Test that newline escapes are only supported in strings. + test_balanced_unparseable("di\\\nv"); + test_balanced_unparseable("div \\\n p"); + test_balanced_unparseable("div\\\n p"); + test_balanced_unparseable("div \\\np"); + test_balanced_unparseable("div\\\np"); + run_deferred_tests(); } From 1681bbfeed6ae9759a3ebb3dae33feaa36d625b1 Mon Sep 17 00:00:00 2001 From: "L. David Baron" Date: Tue, 3 May 2011 13:19:19 -0700 Subject: [PATCH 5/6] Fix the remaining todo test in test_parse_rule.html, which is invalid since ~= selectors never match anything containing spaces. (Bug 384672, patch 5) r=bzbarsky --- layout/style/test/test_parse_rule.html | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/layout/style/test/test_parse_rule.html b/layout/style/test/test_parse_rule.html index 78cd8c36c9e..338d6541924 100644 --- a/layout/style/test/test_parse_rule.html +++ b/layout/style/test/test_parse_rule.html @@ -48,7 +48,7 @@ base + "#a {color: rgb\n(255, 0, 0)}", "@threedee maroon url('asdf\n) ra('asdf\n); " + base, "@threedee {maroon url('asdf\n) ra('asdf\n);} " + base, "div[title='zxcv weeqweqeweasd\\D\\A a']{color:green}", -"div[title~='weeqweqeweasd\\D\\A a']{color:green}", +"div[title~='weeqweqeweasd']{color:green}", base + "#a\\\n{color:red}", base + "#a\v{color:red}", @@ -129,10 +129,7 @@ base + "#a {color: rgb(255.0, 0, 0)}", "#a {color: rgb(0, 128, 0)}", "#a {color: rgb(0%, 50%, 0%)}", "#a {color: rgb(0%, 49.999999999999%, 0%)}", -], prop: "color", pseudo: "", -todo: { - "div[title~='weeqweqeweasd\\D\\A a']{color:green}" : 1 - } +], prop: "color", pseudo: "" }, // Border tests From 3e42b28f7b5e9b154b913b4c6e948c1f462738bd Mon Sep 17 00:00:00 2001 From: Johnny Stenback Date: Tue, 26 Apr 2011 11:36:38 -0700 Subject: [PATCH 6/6] Fix bug 649795. Crash when accessing mOwner which might not always be set. r=bsmedberg --- dom/plugins/base/nsPluginStreamListenerPeer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/plugins/base/nsPluginStreamListenerPeer.cpp b/dom/plugins/base/nsPluginStreamListenerPeer.cpp index 5cdf9d4dc2b..47c19780143 100644 --- a/dom/plugins/base/nsPluginStreamListenerPeer.cpp +++ b/dom/plugins/base/nsPluginStreamListenerPeer.cpp @@ -874,7 +874,7 @@ nsresult nsPluginStreamListenerPeer::ServeStreamAsFile(nsIRequest *request, window->window = widget->GetNativeData(NS_NATIVE_PLUGIN_PORT); } #endif - mOwner->SetWindow(); + owner->SetWindow(); } mSeekable = PR_FALSE;