- Revision
- 211763
- Author
- bfulg...@apple.com
- Date
- 2017-02-06 17:48:22 -0800 (Mon, 06 Feb 2017)
Log Message
Correct File Path Handling in SecurityOrigin and FileSystem
https://bugs.webkit.org/show_bug.cgi?id=167894
<rdar://problem/30380080>
Reviewed by Alexey Proskuryakov.
Source/WebCore:
Roll out the URL decoding being done in the FileSystem class (added in Bug 167894), and instead ensure that
SecurityOrigin properly handles file URLs, and only passes valid file strings to the FileSystem interface.
Tested by FileSystemTests and SecurityOriginTests in TestWebKitAPI.
* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::SecurityOrigin): Initialize m_filePath using the url's fileSystemPath, not
the %-encoded 'path' property.
(WebCore::SecurityOrigin::canDisplay): Pass the 'fileSystemPath' to 'filesHaveSameVolume', rather than
the %-encoded 'path' property.
* page/SecurityOrigin.h:
* platform/FileSystem.cpp:
(WebCore::filesHaveSameVolume): Do not use 'decodeURLEscapeSequences' in 'filesHaveSameVolume'.
Tools:
* TestWebKitAPI/Tests/WebCore/FileSystem.cpp: Don't encode the temporary files,
and perform same-volume checks using filesystem-compatible paths.
* TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp: Create SecurityOrigins from
filesystem paths, and perform validation of same-volume checks.
(TestWebKitAPI::SecurityOriginTest::tempFilePath): Added.
(TestWebKitAPI::SecurityOriginTest::spaceContainingFilePath): Added.
(TestWebKitAPI::SecurityOriginTest::bangContainingFilePath): Added.
(TestWebKitAPI::SecurityOriginTest::quoteContainingFilePath): Added.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (211762 => 211763)
--- trunk/Source/WebCore/ChangeLog 2017-02-07 01:39:45 UTC (rev 211762)
+++ trunk/Source/WebCore/ChangeLog 2017-02-07 01:48:22 UTC (rev 211763)
@@ -1,3 +1,25 @@
+2017-02-06 Brent Fulgham <bfulg...@apple.com>
+
+ Correct File Path Handling in SecurityOrigin and FileSystem
+ https://bugs.webkit.org/show_bug.cgi?id=167894
+ <rdar://problem/30380080>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Roll out the URL decoding being done in the FileSystem class (added in Bug 167894), and instead ensure that
+ SecurityOrigin properly handles file URLs, and only passes valid file strings to the FileSystem interface.
+
+ Tested by FileSystemTests and SecurityOriginTests in TestWebKitAPI.
+
+ * page/SecurityOrigin.cpp:
+ (WebCore::SecurityOrigin::SecurityOrigin): Initialize m_filePath using the url's fileSystemPath, not
+ the %-encoded 'path' property.
+ (WebCore::SecurityOrigin::canDisplay): Pass the 'fileSystemPath' to 'filesHaveSameVolume', rather than
+ the %-encoded 'path' property.
+ * page/SecurityOrigin.h:
+ * platform/FileSystem.cpp:
+ (WebCore::filesHaveSameVolume): Do not use 'decodeURLEscapeSequences' in 'filesHaveSameVolume'.
+
2017-02-06 Andy Estes <aes...@apple.com>
[Cocoa] Split FileSystemMac.mm into Mac and Cocoa parts
Modified: trunk/Source/WebCore/page/SecurityOrigin.cpp (211762 => 211763)
--- trunk/Source/WebCore/page/SecurityOrigin.cpp 2017-02-07 01:39:45 UTC (rev 211762)
+++ trunk/Source/WebCore/page/SecurityOrigin.cpp 2017-02-07 01:48:22 UTC (rev 211763)
@@ -114,7 +114,7 @@
m_canLoadLocalResources = isLocal();
if (m_canLoadLocalResources)
- m_filePath = url.path(); // In case enforceFilePathSeparation() is called.
+ m_filePath = url.fileSystemPath(); // In case enforceFilePathSeparation() is called.
}
SecurityOrigin::SecurityOrigin()
@@ -305,7 +305,7 @@
return true;
#if !PLATFORM(IOS)
- if (m_protocol == "file" && url.isLocalFile() && !filesHaveSameVolume(m_filePath, url.path()))
+ if (m_protocol == "file" && url.isLocalFile() && !filesHaveSameVolume(m_filePath, url.fileSystemPath()))
return false;
#endif
Modified: trunk/Source/WebCore/platform/FileSystem.cpp (211762 => 211763)
--- trunk/Source/WebCore/platform/FileSystem.cpp 2017-02-07 01:39:45 UTC (rev 211762)
+++ trunk/Source/WebCore/platform/FileSystem.cpp 2017-02-07 01:48:22 UTC (rev 211763)
@@ -28,7 +28,6 @@
#include "FileSystem.h"
#include "ScopeGuard.h"
-#include "URL.h"
#include <wtf/HexNumber.h>
#include <wtf/text/CString.h>
#include <wtf/text/StringBuilder.h>
@@ -237,8 +236,8 @@
bool filesHaveSameVolume(const String& fileA, const String& fileB)
{
- auto fsRepFileA = fileSystemRepresentation(decodeURLEscapeSequences(fileA));
- auto fsRepFileB = fileSystemRepresentation(decodeURLEscapeSequences(fileB));
+ auto fsRepFileA = fileSystemRepresentation(fileA);
+ auto fsRepFileB = fileSystemRepresentation(fileB);
if (fsRepFileA.isNull() || fsRepFileB.isNull())
return false;
Modified: trunk/Tools/ChangeLog (211762 => 211763)
--- trunk/Tools/ChangeLog 2017-02-07 01:39:45 UTC (rev 211762)
+++ trunk/Tools/ChangeLog 2017-02-07 01:48:22 UTC (rev 211763)
@@ -1,3 +1,20 @@
+2017-02-06 Brent Fulgham <bfulg...@apple.com>
+
+ Correct File Path Handling in SecurityOrigin and FileSystem
+ https://bugs.webkit.org/show_bug.cgi?id=167894
+ <rdar://problem/30380080>
+
+ Reviewed by Alexey Proskuryakov.
+
+ * TestWebKitAPI/Tests/WebCore/FileSystem.cpp: Don't encode the temporary files,
+ and perform same-volume checks using filesystem-compatible paths.
+ * TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp: Create SecurityOrigins from
+ filesystem paths, and perform validation of same-volume checks.
+ (TestWebKitAPI::SecurityOriginTest::tempFilePath): Added.
+ (TestWebKitAPI::SecurityOriginTest::spaceContainingFilePath): Added.
+ (TestWebKitAPI::SecurityOriginTest::bangContainingFilePath): Added.
+ (TestWebKitAPI::SecurityOriginTest::quoteContainingFilePath): Added.
+
2017-02-06 Jer Noble <jer.no...@apple.com>
Playback stalls when a SourceBuffer append causes frame eviction
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/FileSystem.cpp (211762 => 211763)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/FileSystem.cpp 2017-02-07 01:39:45 UTC (rev 211762)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/FileSystem.cpp 2017-02-07 01:48:22 UTC (rev 211763)
@@ -28,7 +28,6 @@
#include "Test.h"
#include <WebCore/FileSystem.h>
-#include <WebCore/URL.h>
#include <wtf/MainThread.h>
#include <wtf/StringExtras.h>
@@ -54,15 +53,15 @@
m_tempEmptyFilePath = openTemporaryFile("tempEmptyTestFile", handle);
closeFile(handle);
- m_spaceContainingFilePath = encodeWithURLEscapeSequences(openTemporaryFile("temp Empty Test File", handle));
+ m_spaceContainingFilePath = openTemporaryFile("temp Empty Test File", handle);
closeFile(handle);
- m_bangContainingFilePath = encodeWithURLEscapeSequences(openTemporaryFile("temp!Empty!Test!File", handle));
+ m_bangContainingFilePath = openTemporaryFile("temp!Empty!Test!File", handle);
closeFile(handle);
- m_quoteContainingFilePath = encodeWithURLEscapeSequences(openTemporaryFile("temp\"Empty\"TestFile", handle));
+ m_quoteContainingFilePath = openTemporaryFile("temp\"Empty\"TestFile", handle);
closeFile(handle);
-}
+ }
void TearDown() override
{
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp (211762 => 211763)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp 2017-02-07 01:39:45 UTC (rev 211762)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp 2017-02-07 01:48:22 UTC (rev 211763)
@@ -25,6 +25,7 @@
#include "config.h"
#include "WTFStringUtilities.h"
+#include <WebCore/FileSystem.h>
#include <WebCore/SecurityOrigin.h>
#include <WebCore/URL.h>
#include <wtf/MainThread.h>
@@ -37,7 +38,40 @@
public:
void SetUp() final {
WTF::initializeMainThread();
+
+ // create temp file
+ PlatformFileHandle handle;
+ m_tempFilePath = openTemporaryFile("tempTestFile", handle);
+ closeFile(handle);
+
+ m_spaceContainingFilePath = openTemporaryFile("temp Empty Test File", handle);
+ closeFile(handle);
+
+ m_bangContainingFilePath = openTemporaryFile("temp!Empty!Test!File", handle);
+ closeFile(handle);
+
+ m_quoteContainingFilePath = openTemporaryFile("temp\"Empty\"TestFile", handle);
+ closeFile(handle);
}
+
+ void TearDown() override
+ {
+ deleteFile(m_tempFilePath);
+ deleteFile(m_spaceContainingFilePath);
+ deleteFile(m_bangContainingFilePath);
+ deleteFile(m_quoteContainingFilePath);
+ }
+
+ const String& tempFilePath() { return m_tempFilePath; }
+ const String& spaceContainingFilePath() { return m_spaceContainingFilePath; }
+ const String& bangContainingFilePath() { return m_bangContainingFilePath; }
+ const String& quoteContainingFilePath() { return m_quoteContainingFilePath; }
+
+private:
+ String m_tempFilePath;
+ String m_spaceContainingFilePath;
+ String m_bangContainingFilePath;
+ String m_quoteContainingFilePath;
};
TEST_F(SecurityOriginTest, SecurityOriginConstructors)
@@ -84,4 +118,29 @@
EXPECT_TRUE(o1->isSameOriginAs(o6.get()));
}
+TEST_F(SecurityOriginTest, SecurityOriginFileBasedConstructors)
+{
+ auto tempFileOrigin = SecurityOrigin::create(URL(URL(), "file:///" + tempFilePath()));
+ auto spaceContainingOrigin = SecurityOrigin::create(URL(URL(), "file:///" + spaceContainingFilePath()));
+ auto bangContainingOrigin = SecurityOrigin::create(URL(URL(), "file:///" + bangContainingFilePath()));
+ auto quoteContainingOrigin = SecurityOrigin::create(URL(URL(), "file:///" + quoteContainingFilePath()));
+
+ EXPECT_EQ(String("file"), tempFileOrigin->protocol());
+ EXPECT_EQ(String("file"), spaceContainingOrigin->protocol());
+ EXPECT_EQ(String("file"), bangContainingOrigin->protocol());
+ EXPECT_EQ(String("file"), quoteContainingOrigin->protocol());
+
+ EXPECT_TRUE(tempFileOrigin->isSameOriginAs(spaceContainingOrigin.get()));
+ EXPECT_TRUE(tempFileOrigin->isSameOriginAs(bangContainingOrigin.get()));
+ EXPECT_TRUE(tempFileOrigin->isSameOriginAs(quoteContainingOrigin.get()));
+
+ EXPECT_TRUE(tempFileOrigin->isSameSchemeHostPort(spaceContainingOrigin.get()));
+ EXPECT_TRUE(tempFileOrigin->isSameSchemeHostPort(bangContainingOrigin.get()));
+ EXPECT_TRUE(tempFileOrigin->isSameSchemeHostPort(quoteContainingOrigin.get()));
+
+ EXPECT_TRUE(tempFileOrigin->canAccess(spaceContainingOrigin.get()));
+ EXPECT_TRUE(tempFileOrigin->canAccess(bangContainingOrigin.get()));
+ EXPECT_TRUE(tempFileOrigin->canAccess(quoteContainingOrigin.get()));
+}
+
} // namespace TestWebKitAPI
