Ampersands in URL aren't escaped when inserted in generated base attribute

VERIFIED FIXED in Firefox 3 alpha1

Status

()

VERIFIED FIXED
12 years ago
10 years ago

People

(Reporter: svl-bmo, Assigned: sayrer)

Tracking

({verified1.8.1.1})

unspecified
Firefox 3 alpha1
x86
Windows 2000
verified1.8.1.1
Points:
---
Bug Flags:
blocking1.8.1.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

12 years ago
Loading the atom feed at http://www.travellerspoint.com/photo_gallery_feed.cfm?tags=Canada&onlyShowFeatured=true the feed preview screen shows blank (feed subscription block, title "Canada - Travellerspoint Travel Photo Gallery : Featured Photos" and then nothing), whereas http://www.travellerspoint.com/photo_gallery_feed.cfm?tags=Canada shows a lot of photos.

The ever helpful error console shows 5 errors:
==============================================
Error: not well-formed
Source File: 
Line: 1, Column: 135
Source Code:
<div xmlns="http://www.w3.org/1999/xhtml" xml:base="http://www.travellerspoint.com/photo_gallery_feed.cfm?tags=Canada&onlyShowFeatured=true">--------------------------------------------------------------------------------------------------------------------------------------^

XML Parsing Error: not well-formed
Location: 
Line Number 1, Column 135:

Error: [Exception... "An invalid or illegal string was specified"  code: "12" nsresult: "0x8053000c (NS_ERROR_DOM_SYNTAX_ERR)"  location: "file:///C:/mozilla/firefox%202.0/components/FeedProcessor.js Line: 514"]
Source File: file:///C:/mozilla/firefox%202.0/components/FeedProcessor.js
Line: 514

Error: [Exception... "An invalid or illegal string was specified"  code: "12" nsresult: "0x8053000c (NS_ERROR_DOM_SYNTAX_ERR)"  location: "file:///C:/mozilla/firefox%202.0/components/FeedProcessor.js Line: 514"]
Source File: file:///C:/mozilla/firefox%202.0/components/FeedProcessor.js
Line: 514

Error: uncaught exception: [Exception... "An invalid or illegal string was specified"  code: "12" nsresult: "0x8053000c (NS_ERROR_DOM_SYNTAX_ERR)"  location: "file:///C:/mozilla/firefox%202.0/components/FeedProcessor.js Line: 514"]
==============================================

The funny thing is - that base attribute isn't present in the source; Firefox is adding it, and then not escaping the ampersand!

Tested with 2.0, with aja in #developers confirming this still to happen on trunk. (I seem to be running into bug 330271 on trunk, so can't test.)
(Assignee)

Updated

12 years ago
Assignee: nobody → sayrer
(Assignee)

Comment 1

12 years ago
Ok, this is a pretty atrocious error on my part, in setting up the tag stack for createDocumentFragment in nsScriptableUnescape. This will only happen with Atom feeds that have XHTML content served from a base containing an ampersand.

Reporter: the workaround is to use type="html" on your text until this is fixed.
Flags: blocking1.8.1.1?
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 2

12 years ago
Created attachment 246310 [details] [diff] [review]
escape the spec on the tagStack
Attachment #246310 - Flags: review?(mano)
(Assignee)

Updated

12 years ago
Target Milestone: --- → Firefox 3 alpha1
Comment on attachment 246310 [details] [diff] [review]
escape the spec on the tagStack

>Index: src/nsScriptableUnescapeHTML.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/feeds/src/nsScriptableUnescapeHTML.cpp,v
>retrieving revision 1.4
>diff -u -8 -p -r1.4 nsScriptableUnescapeHTML.cpp
>--- src/nsScriptableUnescapeHTML.cpp	4 Nov 2006 05:45:02 -0000	1.4
>+++ src/nsScriptableUnescapeHTML.cpp	22 Nov 2006 18:54:25 -0000
>@@ -37,17 +37,17 @@
> #include "nsString.h"
> #include "nsISupportsArray.h"
> #include "nsIComponentManager.h"
> #include "nsCOMPtr.h"
> #include "nsXPCOM.h"
> #include "nsISupportsPrimitives.h"
> #include "nsXPIDLString.h"
> #include "nsScriptLoader.h"
>-
>+#include "nsEscape.h"
> #include "nsIParser.h"
> #include "nsIDTD.h"
> #include "nsNetCID.h"
> #include "nsNetUtil.h"
> #include "nsParserCIID.h"
> #include "nsParserCIID.h"
> #include "nsIContentSink.h"
> #include "nsIHTMLToTextSink.h"
>@@ -152,17 +152,21 @@ nsScriptableUnescapeHTML::ParseFragment(
>   nsVoidArray tagStack;
>   nsCAutoString base, spec;
>   if (aIsXML) {
>     // XHTML
>     if (aBaseURI) {
>       base.Append(NS_LITERAL_CSTRING(XHTML_DIV_TAG));
>       base.Append(NS_LITERAL_CSTRING(" xml:base=\""));
>       aBaseURI->GetSpec(spec);
>-      base = base + spec;
>+      // nsEscapeHTML is good enough, because we only need to get
>+      // quotes, ampersands, and angle brackets
>+      char* escapedSpec = nsEscapeHTML(spec.get());
>+      base += escapedSpec;
>+      NS_Free(escapedSpec);

null-check escapedSpec.

r=mano otherwise assuming you've tested this.
Attachment #246310 - Flags: review?(mano) → review+
(Assignee)

Comment 4

12 years ago
Created attachment 246327 [details] [diff] [review]
patch to check in

added null check

Comment 5

12 years ago
Comment on attachment 246327 [details] [diff] [review]
patch to check in

Assuming this is the correct patch, approving it for 1.8.1 branch.  a=jay for drivers, please land asap.  Thanks!
Attachment #246327 - Flags: approval1.8.1.1+

Updated

12 years ago
Flags: blocking1.8.1.1? → blocking1.8.1.1+
(Assignee)

Comment 6

12 years ago
Checking in src/nsScriptableUnescapeHTML.cpp;
/cvsroot/mozilla/toolkit/components/feeds/src/nsScriptableUnescapeHTML.cpp,v  <--  nsScriptableUnescapeHTML.cpp
new revision: 1.5; previous revision: 1.4
done
RCS file: /cvsroot/mozilla/toolkit/components/feeds/test/xml/rfc4287/entry_xhtml_baseURI_with_amp.xml,v
done
Checking in test/xml/rfc4287/entry_xhtml_baseURI_with_amp.xml;
/cvsroot/mozilla/toolkit/components/feeds/test/xml/rfc4287/entry_xhtml_baseURI_with_amp.xml,v  <--  entry_xhtml_baseURI_with_amp.xml
initial revision: 1.1
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Flags: in-testsuite+
Whiteboard: [checkin needed (1.8 branch)]
(Assignee)

Comment 7

12 years ago
Checking in src/nsScriptableUnescapeHTML.cpp;
/cvsroot/mozilla/toolkit/components/feeds/src/nsScriptableUnescapeHTML.cpp,v  <--  nsScriptableUnescapeHTML.cpp
new revision: 1.1.2.5; previous revision: 1.1.2.4
done
Checking in test/xml/rfc4287/entry_xhtml_baseURI_with_amp.xml;
/cvsroot/mozilla/toolkit/components/feeds/test/xml/rfc4287/entry_xhtml_baseURI_with_amp.xml,v  <--  entry_xhtml_baseURI_with_amp.xml
new revision: 1.1.2.1; previous revision: 1.1
done
Keywords: fixed1.8.1
Whiteboard: [checkin needed (1.8 branch)]
Keywords: fixed1.8.1 → fixed1.8.1.1
(Reporter)

Comment 8

12 years ago
Verified fixed in 20061128, both branch and trunk. Thanks for the swift patch!
Status: RESOLVED → VERIFIED

Comment 9

12 years ago
Replacing "fixed1.8.1.1" keyword with "verified1.8.1.1" to keep track of verifications.
Keywords: fixed1.8.1.1 → verified1.8.1.1
Depends on: 453146
You need to log in before you can comment on or make changes to this bug.