FileReader crashes when file is not readable
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Feb 2012 19:49:54 +0000 (19:49 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Feb 2012 19:49:54 +0000 (19:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79715

Reviewed by Jian Li.

Source/WebCore:

Test: fast/files/file-reader-directory-crash.html

* platform/SharedBuffer.cpp: (WebCore::SharedBuffer::SharedBuffer): Crash early if a caller
mixed up in-band error signal with length again.

* platform/network/BlobResourceHandle.cpp:
(WebCore): Changed errors into an enum. Added a proper domain for blob errors.
(WebCore::BlobResourceHandle::didReceiveResponse): There is already a constant for INT_MAX
in C/C++.
(WebCore::BlobResourceHandle::didRead): Don't send "-1" for failure down the success path.
(WebCore::BlobResourceHandle::notifyFail): Use a proper domain for blob errors, and a non-
empty message.

LayoutTests:

* fast/files/file-reader-directory-crash-expected.txt: Added.
* fast/files/file-reader-directory-crash.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@109132 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/files/file-reader-directory-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/files/file-reader-directory-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/SharedBuffer.cpp
Source/WebCore/platform/network/BlobResourceHandle.cpp

index a36a83d..98882fd 100644 (file)
@@ -1,3 +1,13 @@
+2012-02-28  Alexey Proskuryakov  <ap@apple.com>
+
+        FileReader crashes when file is not readable
+        https://bugs.webkit.org/show_bug.cgi?id=79715
+
+        Reviewed by Jian Li.
+
+        * fast/files/file-reader-directory-crash-expected.txt: Added.
+        * fast/files/file-reader-directory-crash.html: Added.
+
 2012-02-28  Emil A Eklund  <eae@chromium.org>
 
         Fix printing/zoomed-document.html for QT
diff --git a/LayoutTests/fast/files/file-reader-directory-crash-expected.txt b/LayoutTests/fast/files/file-reader-directory-crash-expected.txt
new file mode 100644 (file)
index 0000000..96d0410
--- /dev/null
@@ -0,0 +1,2 @@
+
+PASS, no crash
diff --git a/LayoutTests/fast/files/file-reader-directory-crash.html b/LayoutTests/fast/files/file-reader-directory-crash.html
new file mode 100644 (file)
index 0000000..9550cb4
--- /dev/null
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<body>
+<input type=file id=file onchange='onInputFileChange()'>
+<p id=status>To test manually, drag a directory to the file input above.</p>
+<script>
+var input = document.getElementsByTagName("input")[0];
+var statusElement = document.getElementById("status");
+
+function onInputFileChange()
+{
+    var file = document.getElementById('file').files[0];
+    var reader = new FileReader();
+    statusElement.textContent = "Starting test...";
+    reader.readAsText(file);
+    reader.onloadend = function() {
+        statusElement.textContent = "PASS, no crash";
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }
+}
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+eventSender.beginDragWithFiles(['resources']);
+eventSender.mouseMoveTo(input.offsetLeft + 1, input.offsetTop + 1);
+eventSender.mouseUp();
+
+</script>
+</body>
+</html>
index 3875225..a7f0a50 100644 (file)
@@ -1,3 +1,23 @@
+2012-02-28  Alexey Proskuryakov  <ap@apple.com>
+
+        FileReader crashes when file is not readable
+        https://bugs.webkit.org/show_bug.cgi?id=79715
+
+        Reviewed by Jian Li.
+
+        Test: fast/files/file-reader-directory-crash.html
+
+        * platform/SharedBuffer.cpp: (WebCore::SharedBuffer::SharedBuffer): Crash early if a caller
+        mixed up in-band error signal with length again.
+
+        * platform/network/BlobResourceHandle.cpp:
+        (WebCore): Changed errors into an enum. Added a proper domain for blob errors.
+        (WebCore::BlobResourceHandle::didReceiveResponse): There is already a constant for INT_MAX
+        in C/C++.
+        (WebCore::BlobResourceHandle::didRead): Don't send "-1" for failure down the success path.
+        (WebCore::BlobResourceHandle::notifyFail): Use a proper domain for blob errors, and a non-
+        empty message.
+
 2012-02-28  Adam Klein  <adamk@chromium.org>
 
         Unreviewed, fix cr-win build after r109119.
index 5b5953e..ee6a29f 100644 (file)
@@ -65,12 +65,20 @@ SharedBuffer::SharedBuffer()
 SharedBuffer::SharedBuffer(const char* data, int size)
     : m_size(0)
 {
+    // FIXME: Use unsigned consistently, and check for invalid casts when calling into SharedBuffer from other code.
+    if (size < 0)
+        CRASH();
+
     append(data, size);
 }
 
 SharedBuffer::SharedBuffer(const unsigned char* data, int size)
     : m_size(0)
 {
+    // FIXME: Use unsigned consistently, and check for invalid casts when calling into SharedBuffer from other code.
+    if (size < 0)
+        CRASH();
+
     append(reinterpret_cast<const char*>(data), size);
 }
     
index 2b94bdb..e8ed075 100644 (file)
@@ -65,10 +65,13 @@ static const char* httpNotFoundText = "Not Found";
 static const char* httpRequestedRangeNotSatisfiableText = "Requested Range Not Satisfiable";
 static const char* httpInternalErrorText = "Internal Server Error";
 
-static const int notFoundError = 1;
-static const int securityError = 2;
-static const int rangeError = 3;
-static const int notReadableError = 4;
+static const char* const webKitBlobResourceDomain = "WebKitBlobResource";
+enum {
+    notFoundError = 1,
+    securityError = 2,
+    rangeError = 3,
+    notReadableError = 4,
+};
 
 ///////////////////////////////////////////////////////////////////////////////
 // BlobResourceSynchronousLoader
@@ -100,9 +103,8 @@ BlobResourceSynchronousLoader::BlobResourceSynchronousLoader(ResourceError& erro
 void BlobResourceSynchronousLoader::didReceiveResponse(ResourceHandle* handle, const ResourceResponse& response)
 {
     // We cannot handle the size that is more than maximum integer.
-    const int intMaxForLength = 0x7fffffff;
-    if (response.expectedContentLength() > intMaxForLength) {
-        m_error = ResourceError(String(), notReadableError, response.url(), String());
+    if (response.expectedContentLength() > INT_MAX) {
+        m_error = ResourceError(webKitBlobResourceDomain, notReadableError, response.url(), "File is too large");
         return;
     }
 
@@ -486,6 +488,11 @@ void BlobResourceHandle::didOpen(bool success)
 
 void BlobResourceHandle::didRead(int bytesRead)
 {
+    if (bytesRead < 0) {
+        failed(notReadableError);
+        return;
+    }
+
     consumeData(m_buffer.data(), bytesRead);
 }
 
@@ -592,7 +599,7 @@ void BlobResourceHandle::notifyReceiveData(const char* data, int bytesRead)
 void BlobResourceHandle::notifyFail(int errorCode)
 {
     if (client())
-        client()->didFail(this, ResourceError(String(), errorCode, firstRequest().url(), String()));
+        client()->didFail(this, ResourceError(webKitBlobResourceDomain, errorCode, firstRequest().url(), String()));
 }
 
 static void doNotifyFinish(void* context)