Bug 45565 - URIs in styles created via innerHTML are not resolved against the document's base URI
Summary: URIs in styles created via innerHTML are not resolved against the document's ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other All
: P2 Normal
Assignee: Mihai Parparita
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-10 14:24 PDT by Mihai Parparita
Modified: 2010-09-11 02:00 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.49 KB, patch)
2010-09-10 14:27 PDT, Mihai Parparita
no flags Details | Formatted Diff | Diff
Patch (10.20 KB, patch)
2010-09-10 16:13 PDT, Mihai Parparita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Parparita 2010-09-10 14:24:32 PDT
URIs in styles created via innerHTML are not resolved against the document's base URI
Comment 1 Mihai Parparita 2010-09-10 14:27:03 PDT
Created attachment 67241 [details]
Patch
Comment 2 Mihai Parparita 2010-09-10 14:29:38 PDT
Note that this only affects Chromium right now since GoogleURL canonicalizes after resolving (previously against an empty URL), while KURL leaves the partial URL alone, so it would get re-resolved correctly once the nodes were moved from the fragment's dummy document into the real document.
Comment 3 Mihai Parparita 2010-09-10 14:32:03 PDT
Adam, can you review this?
Comment 4 Adam Barth 2010-09-10 14:40:39 PDT
Comment on attachment 67241 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67241&action=prettypatch

> WebCore/html/parser/HTMLTreeBuilder.cpp:405
> -    : m_dummyDocumentForFragmentParsing(HTMLDocument::create(0, KURL()))
> +    : m_dummyDocumentForFragmentParsing(HTMLDocument::create(0, fragment->document()->baseURI()))
You don't want to set the URL of the dummy document to the baseURL.  That's a potential security vulnerability (since the base URL can be anything the document wants).  It's probably harmless right now, but it's pretty dangerous.  Instead, we want set the baseURL of the m_dummyDocumentForFragmentParsing after we construct it.
Comment 5 Mihai Parparita 2010-09-10 16:13:32 PDT
Created attachment 67256 [details]
Patch
Comment 6 Mihai Parparita 2010-09-10 16:14:36 PDT
(In reply to comment #4)
> You don't want to set the URL of the dummy document to the baseURL.  That's a potential security vulnerability (since the base URL can be anything the document wants).  It's probably harmless right now, but it's pretty dangerous.  Instead, we want set the baseURL of the m_dummyDocumentForFragmentParsing after we construct it.

As discussed over IRC, no setter for the baseURI is exposed (and we probably don't want to have one). Added a basURI constructor parameter to (HTML)Document instead.
Comment 7 Adam Barth 2010-09-10 16:16:38 PDT
Comment on attachment 67256 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67256&action=prettypatch

> WebCore/html/HTMLElement.cpp:-272
> -static bool useLegacyTreeBuilder(Document*)
> -{
> -    return false;
> -}
This patch is good, but it should be separate.
Comment 8 WebKit Commit Bot 2010-09-11 02:00:20 PDT
Comment on attachment 67256 [details]
Patch

Clearing flags on attachment: 67256

Committed r67292: <http://trac.webkit.org/changeset/67292>
Comment 9 WebKit Commit Bot 2010-09-11 02:00:25 PDT
All reviewed patches have been landed.  Closing bug.