2010-06-03 Abhishek Arya <inferno@chromium.org>
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Jun 2010 06:16:55 +0000 (06:16 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Jun 2010 06:16:55 +0000 (06:16 +0000)
        Reviewed by Eric Carlson.

        Tests that invalid media src url does not result in crash.

        * media/invalid-media-url-crash-expected.txt: Added.
        * media/invalid-media-url-crash.html: Added.
        * platform/gtk/Skipped:
        * platform/qt/Skipped:

2010-06-03  Abhishek Arya  <inferno@chromium.org>

        Reviewed by Eric Carlson.

        Fix a crash when trying to use an invalid media src url by
        moving the isValid url checks to a central location in
        isSafeToLoadURL function. Also added an empty string check
        in DocumentLoader::didTellClientAboutLoad.

        Test: media/invalid-media-url-crash.html

        * html/HTMLMediaElement.cpp:
        (WebCore::HTMLMediaElement::isSafeToLoadURL):
        (WebCore::HTMLMediaElement::selectNextSourceChild):
        (WebCore::HTMLMediaElement::getPluginProxyParams):
        * loader/DocumentLoader.h:
        (WebCore::DocumentLoader::didTellClientAboutLoad):

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

LayoutTests/ChangeLog
LayoutTests/media/invalid-media-url-crash-expected.txt [new file with mode: 0644]
LayoutTests/media/invalid-media-url-crash.html [new file with mode: 0644]
LayoutTests/platform/gtk/Skipped
LayoutTests/platform/qt/Skipped
WebCore/ChangeLog
WebCore/html/HTMLMediaElement.cpp
WebCore/loader/DocumentLoader.h

index 549e494725b03508327e46b3cc5e3d77c7464475..421bc2d2f8229d97c0816ed2c16043192478ef91 100644 (file)
@@ -1,3 +1,14 @@
+2010-06-03  Abhishek Arya  <inferno@chromium.org>
+
+        Reviewed by Eric Carlson.
+
+        Tests that invalid media src url does not result in crash.
+
+        * media/invalid-media-url-crash-expected.txt: Added.
+        * media/invalid-media-url-crash.html: Added.
+        * platform/gtk/Skipped:
+        * platform/qt/Skipped:
+
 2010-06-03  Kent Tamura  <tkent@chromium.org>
 
         Unreviewed. Test expectation fix.
diff --git a/LayoutTests/media/invalid-media-url-crash-expected.txt b/LayoutTests/media/invalid-media-url-crash-expected.txt
new file mode 100644 (file)
index 0000000..406d03e
--- /dev/null
@@ -0,0 +1,8 @@
+Tests that invalid media src url does not result in crash.
+
+EXPECTED (audio.error.code == '4') OK
+EXPECTED (video.error.code == '4') OK
+EXPECTED (audio.networkState == '4') OK
+EXPECTED (video.networkState == '4') OK
+END OF TEST
+
diff --git a/LayoutTests/media/invalid-media-url-crash.html b/LayoutTests/media/invalid-media-url-crash.html
new file mode 100644 (file)
index 0000000..73f9be6
--- /dev/null
@@ -0,0 +1,32 @@
+<html>
+    <body>
+        <p>Tests that invalid media src url does not result in crash.</p>
+        <script src=video-test.js></script>
+        <script>
+            var invalid_url = "http:aa" + String.fromCharCode(0) + "%aa#aa";
+            var error_count = 0;
+
+            function errorEvent()
+            {
+                error_count++;
+                if (error_count == 2)
+                {
+                    testExpected("audio.error.code", MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
+                    testExpected("video.error.code", MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
+                    testExpected("audio.networkState", HTMLMediaElement.NETWORK_NO_SOURCE);
+                    testExpected("video.networkState", HTMLMediaElement.NETWORK_NO_SOURCE);
+                    endTest();
+                }
+            }
+
+            var audio = document.createElement('audio');
+            var video = document.createElement('video');
+            audio.src = invalid_url;
+            video.src = invalid_url;
+            audio.onerror = errorEvent;
+            video.onerror = errorEvent;
+            document.body.appendChild(audio);
+            document.body.appendChild(video);
+        </script>
+    </body>
+</html>
index 3fd18282ea80ae7870a27917ac12852a35268d5e..b4f59883a76ae21d880a1f2e8f59fd85493650b6 100644 (file)
@@ -3423,6 +3423,7 @@ media/video-seek-past-end-playing.html
 media/video-source-error.html
 
 #   Tests timing out
+media/invalid-media-url-crash.html
 media/unsupported-rtsp.html
 #   Tests generating new results
 media/audio-controls-rendering.html
index 364d22c6e95f7985341cf22ac873182c73aa7f9d..1eb72959b88973da0eaae93e0107daa4080fc198 100644 (file)
@@ -151,6 +151,7 @@ media/controls-strict.html
 media/controls-styling.html
 media/event-attributes.html
 media/fallback.html
+media/invalid-media-url-crash.html
 media/media-captions.html
 media/media-constants.html
 media/media-fullscreen-inline.html
index 3e5243d055b7d421e82f161c8e6ac4c1d713543c..fae8b2f04f7a8a73f2c545730f79dcb0c9a45eed 100644 (file)
@@ -1,3 +1,21 @@
+2010-06-03  Abhishek Arya  <inferno@chromium.org>
+
+        Reviewed by Eric Carlson.
+
+        Fix a crash when trying to use an invalid media src url by
+        moving the isValid url checks to a central location in
+        isSafeToLoadURL function. Also added an empty string check
+        in DocumentLoader::didTellClientAboutLoad.
+
+        Test: media/invalid-media-url-crash.html
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::isSafeToLoadURL):
+        (WebCore::HTMLMediaElement::selectNextSourceChild):
+        (WebCore::HTMLMediaElement::getPluginProxyParams):
+        * loader/DocumentLoader.h:
+        (WebCore::DocumentLoader::didTellClientAboutLoad):
+
 2010-06-03  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r60642.
index 259d35601859d476eeeac985863ccf5bd28f889a..f5cc88d68f3b6ea87e7e90dfddd92d04505db3b0 100644 (file)
@@ -655,6 +655,9 @@ void HTMLMediaElement::loadResource(const KURL& initialURL, ContentType& content
 
 bool HTMLMediaElement::isSafeToLoadURL(const KURL& url, InvalidSourceAction actionIfInvalid)
 {
+    if (!url.isValid())
+        return false;
+    
     Frame* frame = document()->frame();
     FrameLoader* loader = frame ? frame->loader() : 0;
 
@@ -1482,7 +1485,7 @@ KURL HTMLMediaElement::selectNextSourceChild(ContentType *contentType, InvalidSo
 
         // Is it safe to load this url?
         mediaURL = source->src();
-        if (!mediaURL.isValid() || !isSafeToLoadURL(mediaURL, actionIfInvalid) || !dispatchBeforeLoadEvent(mediaURL.string()))
+        if (!isSafeToLoadURL(mediaURL, actionIfInvalid) || !dispatchBeforeLoadEvent(mediaURL.string()))
             goto check_again;
 
         // Making it this far means the <source> looks reasonable
@@ -1950,7 +1953,7 @@ void HTMLMediaElement::getPluginProxyParams(KURL& url, Vector<String>& names, Ve
     }
 
     url = src();
-    if (!url.isValid() || !isSafeToLoadURL(url, Complain))
+    if (!isSafeToLoadURL(url, Complain))
         url = selectNextSourceChild(0, DoNothing);
 
     m_currentSrc = url.string();
index ccfbfaa308e686a08b133c48501244cd76fd9c94..5cc1cdc463796ff12c5a0a6c942ba41e4f11bba4 100644 (file)
@@ -196,7 +196,11 @@ namespace WebCore {
         void setDeferMainResourceDataLoad(bool defer) { m_deferMainResourceDataLoad = defer; }
         bool deferMainResourceDataLoad() const { return m_deferMainResourceDataLoad; }
         
-        void didTellClientAboutLoad(const String& url) { m_resourcesClientKnowsAbout.add(url); }
+        void didTellClientAboutLoad(const String& url)
+        { 
+            if (!url.isEmpty())
+                m_resourcesClientKnowsAbout.add(url);
+        }
         bool haveToldClientAboutLoad(const String& url) { return m_resourcesClientKnowsAbout.contains(url); }
         void recordMemoryCacheLoadForFutureClientNotification(const String& url);
         void takeMemoryCacheLoadsForClientNotification(Vector<String>& loads);