Reland of Take care of a FIXME in SecurityOrigin.cpp to check the validity...

Reland of Take care of a FIXME in SecurityOrigin.cpp to check the validity (patchset #1 id:1 of https://codereview.chromium.org/1315793002/ )

Reason for revert:
Failure still present.

Original issue's description:
> Revert of Take care of a FIXME in SecurityOrigin.cpp to check the validity of any innerURL during constructio… (patchset #4 id:60001 of https://codereview.chromium.org/1294933004/ )
> 
> Reason for revert:
> Speculative revert for "Too many opened files in the system".
> 
> Original issue's description:
> > Take care of a FIXME in SecurityOrigin.cpp to check the validity of any innerURL during construction and to handle invalidity as a 'unique' origin.
> > 
> > TBR=mkwst
> > BUG=514076
> > 
> > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201022
> 
> TBR=jsbell@chromium.org,palmer@chromium.org,mkwst@chromium.org,brettw@chromium.org,michaeln@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=514076,524248
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201100

TBR=brettw@chromium.org,jsbell@chromium.org,michaeln@chromium.org,mkwst@chromium.org,palmer@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=514076,524248

Review URL: https://codereview.chromium.org/1309333004

git-svn-id: svn://svn.chromium.org/blink/trunk@201102 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent d1bf3366
...@@ -97,19 +97,24 @@ static bool shouldTreatAsUniqueOrigin(const KURL& url) ...@@ -97,19 +97,24 @@ static bool shouldTreatAsUniqueOrigin(const KURL& url)
return true; return true;
// FIXME: Do we need to unwrap the URL further? // FIXME: Do we need to unwrap the URL further?
KURL innerURL = SecurityOrigin::shouldUseInnerURL(url) ? SecurityOrigin::extractInnerURL(url) : url; KURL relevantURL;
if (SecurityOrigin::shouldUseInnerURL(url)) {
// FIXME: Check whether innerURL is valid. relevantURL = SecurityOrigin::extractInnerURL(url);
if (!relevantURL.isValid())
return true;
} else {
relevantURL = url;
}
// For edge case URLs that were probably misparsed, make sure that the origin is unique. // For edge case URLs that were probably misparsed, make sure that the origin is unique.
// FIXME: Do we really need to do this? This looks to be a hack around a // FIXME: Do we really need to do this? This looks to be a hack around a
// security bug in CFNetwork that might have been fixed. // security bug in CFNetwork that might have been fixed.
if (schemeRequiresAuthority(innerURL) && innerURL.host().isEmpty()) if (schemeRequiresAuthority(relevantURL) && relevantURL.host().isEmpty())
return true; return true;
// SchemeRegistry needs a lower case protocol because it uses HashMaps // SchemeRegistry needs a lower case protocol because it uses HashMaps
// that assume the scheme has already been canonicalized. // that assume the scheme has already been canonicalized.
String protocol = innerURL.protocol().lower(); String protocol = relevantURL.protocol().lower();
if (SchemeRegistry::shouldTreatURLSchemeAsNoAccess(protocol)) if (SchemeRegistry::shouldTreatURLSchemeAsNoAccess(protocol))
return true; return true;
......
...@@ -32,6 +32,7 @@ ...@@ -32,6 +32,7 @@
#include "platform/weborigin/SecurityOrigin.h" #include "platform/weborigin/SecurityOrigin.h"
#include "platform/RuntimeEnabledFeatures.h" #include "platform/RuntimeEnabledFeatures.h"
#include "platform/blob/BlobURL.h"
#include "platform/weborigin/KURL.h" #include "platform/weborigin/KURL.h"
#include "platform/weborigin/SecurityPolicy.h" #include "platform/weborigin/SecurityPolicy.h"
#include "wtf/text/StringBuilder.h" #include "wtf/text/StringBuilder.h"
...@@ -385,4 +386,36 @@ TEST_F(SecurityOriginTest, CreateFromTuple) ...@@ -385,4 +386,36 @@ TEST_F(SecurityOriginTest, CreateFromTuple)
} }
TEST_F(SecurityOriginTest, UniquenessPropagatesToBlobUrls)
{
struct TestCase {
const char* url;
bool expectedUniqueness;
const char* expectedOriginString;
} cases[] {
{ "", true, "null" },
{ "null", true, "null" },
{ "data:text/plain,hello_world", true, "null" },
{ "file:///path", false, "file://" },
{ "filesystem:http://host/filesystem-path", false, "http://host" },
{ "filesystem:file:///filesystem-path", false, "file://" },
{ "filesystem:null/filesystem-path", true, "null" },
{ "blob:http://host/blob-id", false, "http://host" },
{ "blob:file:///blob-id", false, "file://" },
{ "blob:null/blob-id", true, "null" },
};
for (const TestCase& test : cases) {
RefPtr<SecurityOrigin> origin = SecurityOrigin::createFromString(test.url);
EXPECT_EQ(test.expectedUniqueness, origin->isUnique());
EXPECT_EQ(test.expectedOriginString, origin->toString());
KURL blobUrl = BlobURL::createPublicURL(origin.get());
RefPtr<SecurityOrigin> blobUrlOrigin = SecurityOrigin::create(blobUrl);
EXPECT_EQ(blobUrlOrigin->isUnique(), origin->isUnique());
EXPECT_EQ(blobUrlOrigin->toString(), origin->toString());
EXPECT_EQ(blobUrlOrigin->toRawString(), origin->toRawString());
}
}
} // namespace blink } // namespace blink
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment