Changeset 205306 in webkit


Ignore:
Timestamp:
Sep 1, 2016 12:27:57 PM (8 years ago)
Author:
commit-queue@webkit.org
Message:

YouTube Flash plug-in replacement facility should more gracefully handle malformed queries
​https://bugs.webkit.org/show_bug.cgi?id=161476
<rdar://problem/28050847>

Patch by Ricky Mondello <Ricky Mondello> on 2016-09-01
Reviewed by Eric Carlson.

Source/WebCore:

Some YouTube Flash embeds use '&' instead of '?' to start the query portion of the URL. Before this patch,
our implementation discards all parts of the path after the '&', which could drop important query information
like the start time for the video. This patch treats anything after that '&' as a "malformed query" and uses
it as the query to restore to the transformed URL if there was no actual query in the original URL.

  • Modules/plugins/YouTubePluginReplacement.cpp:

(WebCore::processAndCreateYouTubeURL): Add an out-parameter for the path after the first ampersand.
(WebCore::YouTubePluginReplacement::youTubeURLFromAbsoluteURL): If the input URL had no query, append

the possibly malformed one found after the first ampersand to the replacement URL.

Tools:

  • TestWebKitAPI/Tests/WebCore/YouTubePluginReplacement.cpp:

(TestWebKitAPI::TEST_F): New tests. The first two and second-to-last test cases cover the "malformed" query

logic. A few other tests are added, too.

Location:
trunk
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r205302 r205306  
     12016-09-01  Ricky Mondello  <rmondello@apple.com>
     2
     3        YouTube Flash plug-in replacement facility should more gracefully handle malformed queries
     4        https://bugs.webkit.org/show_bug.cgi?id=161476
     5        <rdar://problem/28050847>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Some YouTube Flash embeds use '&' instead of '?' to start the query portion of the URL. Before this patch,
     10        our implementation discards all parts of the path after the '&', which could drop important query information
     11        like the start time for the video. This patch treats anything after that '&' as a "malformed query" and uses
     12        it as the query to restore to the transformed URL if there was no actual query in the original URL.
     13
     14        * Modules/plugins/YouTubePluginReplacement.cpp:
     15        (WebCore::processAndCreateYouTubeURL): Add an out-parameter for the path after the first ampersand.
     16        (WebCore::YouTubePluginReplacement::youTubeURLFromAbsoluteURL): If the input URL had no query, append
     17            the possibly malformed one found after the first ampersand to the replacement URL.
     18
    1192016-09-01  Alex Christensen  <achristensen@webkit.org>
    220

  • trunk/Source/WebCore/Modules/plugins/YouTubePluginReplacement.cpp

    r205271 r205306  
    201201}
    202202
    203 static URL processAndCreateYouTubeURL(const URL& url, bool& isYouTubeShortenedURL)
     203static URL processAndCreateYouTubeURL(const URL& url, bool& isYouTubeShortenedURL, String& outPathAfterFirstAmpersand)
    204204{
    205205    if (!url.protocolIsInHTTPFamily())
    … …  
    267267        }
    268268    } else if (hasCaseInsensitivePrefix(path, "/v/") || hasCaseInsensitivePrefix(path, "/e/")) {
    269         String videoID = url.lastPathComponent();
    270        
    271         // These URLs are funny - they don't have a ? for the first query parameter.
    272         // Strip all characters after and including '&' to remove extraneous parameters after the video ID.
    273         size_t ampersand = videoID.find('&');
    274         if (ampersand != notFound)
    275             videoID = videoID.substring(0, ampersand);
    276        
    277         if (!videoID.isEmpty())
     269        String lastPathComponent = url.lastPathComponent();
     270        String videoID;
     271        String pathAfterFirstAmpersand;
     272
     273        size_t ampersandLocation = lastPathComponent.find('&');
     274        if (ampersandLocation != notFound) {
     275            // Some URLs we care about use & in place of ? for the first query parameter.
     276            videoID = lastPathComponent.substring(0, ampersandLocation);
     277            pathAfterFirstAmpersand = lastPathComponent.substring(ampersandLocation + 1, lastPathComponent.length() - ampersandLocation);
     278        } else
     279            videoID = lastPathComponent;
     280
     281        if (!videoID.isEmpty()) {
     282            outPathAfterFirstAmpersand = pathAfterFirstAmpersand;
    278283            return createYouTubeURL(videoID, emptyString());
     284        }
    279285    }
    280286   
    … …  
    291297{
    292298    bool isYouTubeShortenedURL = false;
    293     URL youTubeURL = processAndCreateYouTubeURL(srcURL, isYouTubeShortenedURL);
     299    String possibleMalformedQuery;
     300    URL youTubeURL = processAndCreateYouTubeURL(srcURL, isYouTubeShortenedURL, possibleMalformedQuery);
    294301    if (srcURL.isEmpty() || youTubeURL.isEmpty())
    295302        return srcString;
    … …  
    317324    const String& srcURLPrefix = srcString.substring(0, locationOfPathBeforeVideoID);
    318325    String query = srcURL.query();
     326    // If the URL has no query, use the possibly malformed query we found.
     327    if (query.isEmpty())
     328        query = possibleMalformedQuery;
    319329
    320330    // By default, the iframe will display information like the video title and uploader on top of the video. Don't display
    … …  
    325335        query = "showinfo=0";
    326336   
    327     // Append the query string if it is valid. Some sites apparently forget to add "?" for the query string, in that case,
    328     // we will discard the parameters in the url.
    329     // See: <rdar://problem/11535155>
     337    // Append the query string if it is valid.
    330338    StringBuilder finalURL;
    331339    if (isYouTubeShortenedURL)

  • trunk/Tools/ChangeLog

    r205303 r205306  
     12016-09-01  Ricky Mondello  <rmondello@apple.com>
     2
     3        YouTube Flash plug-in replacement facility should more gracefully handle malformed queries
     4        https://bugs.webkit.org/show_bug.cgi?id=161476
     5        <rdar://problem/28050847>
     6
     7        Reviewed by Eric Carlson.
     8
     9        * TestWebKitAPI/Tests/WebCore/YouTubePluginReplacement.cpp:
     10        (TestWebKitAPI::TEST_F): New tests. The first two and second-to-last test cases cover the "malformed" query
     11            logic. A few other tests are added, too.
     12
    1132016-09-01  Jer Noble  <jer.noble@apple.com>
    214

  • trunk/Tools/TestWebKitAPI/Tests/WebCore/YouTubePluginReplacement.cpp

    r205212 r205306  
    6262    EXPECT_TRUE(test("http://www.youtube.com/v/dQw4w9WgXcQ?start=4&fs=1", "http://www.youtube.com/embed/dQw4w9WgXcQ?start=4&fs=1&showinfo=0"));
    6363
     64    // With an invalid query (see & instead of ?), we preserve and fix the query.
     65    EXPECT_TRUE(test("http://www.youtube.com/v/dQw4w9WgXcQ&start=4", "http://www.youtube.com/embed/dQw4w9WgXcQ?start=4&showinfo=0"));
     66    EXPECT_TRUE(test("http://www.youtube.com/v/dQw4w9WgXcQ&start=4&fs=1", "http://www.youtube.com/embed/dQw4w9WgXcQ?start=4&fs=1&showinfo=0"));
     67
    6468    // Non-Flash URL is untouched.
    6569    EXPECT_TRUE(test("https://www.youtube.com/embed/dQw4w9WgXcQ", "https://www.youtube.com/embed/dQw4w9WgXcQ"));
    66     // Even with an extra parameter.
     70    EXPECT_TRUE(test("http://www.youtube.com/embed/dQw4w9WgXcQ", "http://www.youtube.com/embed/dQw4w9WgXcQ"));
     71    EXPECT_TRUE(test("https://youtube.com/embed/dQw4w9WgXcQ", "https://youtube.com/embed/dQw4w9WgXcQ"));
     72    EXPECT_TRUE(test("http://youtube.com/embed/dQw4w9WgXcQ", "http://youtube.com/embed/dQw4w9WgXcQ"));
     73    // Even with extra parameters.
     74    EXPECT_TRUE(test("https://www.youtube.com/embed/dQw4w9WgXcQ?start=4", "https://www.youtube.com/embed/dQw4w9WgXcQ?start=4"));
    6775    EXPECT_TRUE(test("http://www.youtube.com/embed/dQw4w9WgXcQ?enablejsapi=1", "http://www.youtube.com/embed/dQw4w9WgXcQ?enablejsapi=1"));
     76    // Even with an invalid "query".
     77    EXPECT_TRUE(test("https://www.youtube.com/embed/dQw4w9WgXcQ&start=4", "https://www.youtube.com/embed/dQw4w9WgXcQ&start=4"));
     78
     79    // Don't transform anything with a non "/v/" path component immediately following the domain.
     80    EXPECT_TRUE(test("https://www.youtube.com/something/v/dQw4w9WgXcQ", "https://www.youtube.com/something/v/dQw4w9WgXcQ"));
    6881
    6982    // Non-YouTube domain whose path looks like a Flash video shouldn't be transformed.
Note: See TracChangeset for help on using the changeset viewer.