Bug 23508 – Returns Blank Page for all "about" protocols

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Genevieve Mak]

loader/MainResourceLoader.cpp:
The function shouldLoadAsEmptyDocument() returns true for all "about" prefixed pages (eg "about:config") pages which blocks the implementation of the about protocol. Only "about:blank" should return true for a blank page.

I've included a patch and a log file.
2009-01-23  Genevieve Mak  <[e-mail]>

        Reviewed by NOBODY (OOPS!).

        Fix shouldLoadAsEmptyDocument() to return a blank page for an empty
        url or about:blamk

        * loader/MainResourceLoader.cpp:

diff --git a/WebCore/loader/MainResourceLoader.cpp b/WebCore/loader/MainResourceLoader.cpp
index 325809b..57c9426 100644
--- a/WebCore/loader/MainResourceLoader.cpp
+++ b/WebCore/loader/MainResourceLoader.cpp
@@ -186,7 +186,7 @@ void MainResourceLoader::willSendRequest(ResourceRequest& newRequest, const Reso

 static bool shouldLoadAsEmptyDocument(const KURL& url)
 {
-    return url.isEmpty() || equalIgnoringCase(String(url.protocol()), "about");
+    return url.isEmpty() || (equalIgnoringCase(url.protocol(), "about") && equalIgnoringCase(url.path().stripWhiteSpace(), "blank"));
 }

 void MainResourceLoader::continueAfterContentPolicy(PolicyAction contentPolicy, const ResourceResponse& r)

[Genevieve Mak]

Created attachment 26980 [details]
Patch and logfile

Proposed patch to return a blank page for only "about:blank" urls and empty urls instead of all "about:*" urls.

[Darin Adler]

Comment on attachment 26980 [details]
Patch and logfile

Looks good.

1) The check for "about" should be using the protocolIs function, which is more efficient than calling .protocol() and then calling equalIgnoringCase. You're not changing that aspect of the function, but I'm surprised to see any code still doing it this old-fashioned way.

2) To limit this to "about:blank", then I think the normal idiom would be "equalIgnoringRef(url, blankURL())". This would be more efficient than the code you wrote. Your code would also allow strings like "about: blank " and "about:blank?notsoblank"; maybe that additional behavior is good, maybe bad. You

3) SecurityOrigin.cpp assumes that the "about" protocol has certain security properties. If you implement something that makes some about URLs return something other than an empty page, you could create security problems. I don't know anything more than that.

4) Safari and other Mac OS X applications may well rely on all "about:" URLs giving you a blank document, not just "about:blank", so I think this change may not be safe to make cross-platform.

So I'm going to say review- until you either put this inside some kind of #if or provide some evidence that clients don't rely on this behavior on Mac OS X.

[Genevieve Mak]

Created attachment 27051 [details]
Updated Patch

Thanks, fixed issues #1 and #2. For issue #4 I've also put an "#if PLATFORM() around the fix in order to preserve the status quo. For #3 wouldn't it be up to whomever implements the browser to handle? I could see why you might want default blank pages for the "about" protocol but I'm not sure MainResourceLoader is the right place. When using Qt, MainResourceLoader gets a hold of the request before  QNetworkReplyHandler even gets a chance to let the browser look at it.

[George Staikos]

Comment on attachment 27051 [details]
Updated Patch

Seems to have addressed the issues.  Looks okay to me.

[George Staikos]

Applied in r40491