Changeset 139239 in webkit


Ignore:
Timestamp:
Jan 9, 2013 2:03:48 PM (11 years ago)
Author:
danw@gnome.org
Message:

[Soup] Handle redirection inside WebKit
​https://bugs.webkit.org/show_bug.cgi?id=61122
​https://bugs.webkit.org/show_bug.cgi?id=88961

Reviewed by Martin Robinson.

Source/WebCore:

Rather than using libsoup's built-in redirection handling (which
doesn't do everything exactly the way WebKit wants, and can't
handle redirects to non-http URIs anyway), process redirections
ourselves.

No new tests; unskips a few existing tests.

  • platform/network/ResourceHandleInternal.h:

(WebCore::ResourceHandleInternal::ResourceHandleInternal):
(ResourceHandleInternal):

  • platform/network/soup/ResourceError.h:

(ResourceError):

  • platform/network/soup/ResourceErrorSoup.cpp:

(WebCore::ResourceError::transportError):
(WebCore):
(WebCore::ResourceError::httpError):

  • platform/network/soup/ResourceHandleSoup.cpp:

(WebCore):
(WebCore::gotHeadersCallback):
(WebCore::restartedCallback):
(WebCore::shouldRedirect):
(WebCore::doRedirect):
(WebCore::redirectCloseCallback):
(WebCore::redirectSkipCallback):
(WebCore::cleanupSoupRequestOperation):
(WebCore::sendRequestCallback):
(WebCore::createSoupMessageForHandleAndRequest):
(WebCore::createSoupRequestAndMessageForHandle):
(WebCore::ResourceHandle::start):

LayoutTests:

Unskip some tests.

  • platform/efl/TestExpectations:
  • platform/gtk/TestExpectations:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r139238 r139239  
     12013-01-09  Dan Winship  <danw@gnome.org>
     2
     3        [Soup] Handle redirection inside WebKit
     4        https://bugs.webkit.org/show_bug.cgi?id=61122
     5        https://bugs.webkit.org/show_bug.cgi?id=88961
     6
     7        Reviewed by Martin Robinson.
     8
     9        Unskip some tests.
     10
     11        * platform/efl/TestExpectations:
     12        * platform/gtk/TestExpectations:
     13
    1142013-01-09  Florin Malita  <fmalita@chromium.org>
    215

  • trunk/LayoutTests/platform/efl/TestExpectations

    r139190 r139239  
    9696http/tests/media/video-play-stall.html
    9797http/tests/security/feed-urls-from-remote.html
    98 
    99 # Test times out (via GTK+)
    100 # https://bugs.webkit.org/show_bug.cgi?id=61122
    101 http/tests/navigation/post-307-response.html
    10298
    10399# ------------------------------
    … …  
    13771373webkit.org/b/88138 http/tests/media/video-buffered.html [ Failure ]
    13781374
    1379 # Failing after r120145 on both GTK and EFL
    1380 webkit.org/b/88961 http/tests/misc/redirect-to-about-blank.html [ Failure ]
    1381 
    13821375# EFL port does not support hyphenation
    13831376webkit.org/b/89830 fast/text/hyphen-min-preferred-width.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r139176 r139239  
    907907# Needs to make sure the redirect-chain scenario in https://bugs.webkit.org/show_bug.cgi?id=31410 works
    908908webkit.org/b/35300 http/tests/loading/307-after-303-after-post.html [ Failure ]
    909 webkit.org/b/35300 http/tests/navigation/post-307-response.html [ Skip ]
    910909
    911910# Paged continuousMouseScrollBy() events are not supported in GTK.
    … …  
    12311230webkit.org/b/86771 fast/dom/gc-10.html [ Skip ]
    12321231
    1233 # Failing after r120145 on both GTK and EFL
    1234 webkit.org/b/88961 http/tests/misc/redirect-to-about-blank.html [ Failure ]
    1235 
    12361232webkit.org/b/86971 svg/hittest/svg-shapes-non-scale-stroke.html [ Failure ]
    12371233

  • trunk/Source/WebCore/ChangeLog

    r139238 r139239  
     12013-01-09  Dan Winship  <danw@gnome.org>
     2
     3        [Soup] Handle redirection inside WebKit
     4        https://bugs.webkit.org/show_bug.cgi?id=61122
     5        https://bugs.webkit.org/show_bug.cgi?id=88961
     6
     7        Reviewed by Martin Robinson.
     8
     9        Rather than using libsoup's built-in redirection handling (which
     10        doesn't do everything exactly the way WebKit wants, and can't
     11        handle redirects to non-http URIs anyway), process redirections
     12        ourselves.
     13
     14        No new tests; unskips a few existing tests.
     15
     16        * platform/network/ResourceHandleInternal.h:
     17        (WebCore::ResourceHandleInternal::ResourceHandleInternal):
     18        (ResourceHandleInternal):
     19        * platform/network/soup/ResourceError.h:
     20        (ResourceError):
     21        * platform/network/soup/ResourceErrorSoup.cpp:
     22        (WebCore::ResourceError::transportError):
     23        (WebCore):
     24        (WebCore::ResourceError::httpError):
     25        * platform/network/soup/ResourceHandleSoup.cpp:
     26        (WebCore):
     27        (WebCore::gotHeadersCallback):
     28        (WebCore::restartedCallback):
     29        (WebCore::shouldRedirect):
     30        (WebCore::doRedirect):
     31        (WebCore::redirectCloseCallback):
     32        (WebCore::redirectSkipCallback):
     33        (WebCore::cleanupSoupRequestOperation):
     34        (WebCore::sendRequestCallback):
     35        (WebCore::createSoupMessageForHandleAndRequest):
     36        (WebCore::createSoupRequestAndMessageForHandle):
     37        (WebCore::ResourceHandle::start):
     38
    1392013-01-09  Florin Malita  <fmalita@chromium.org>
    240

  • trunk/Source/WebCore/platform/network/ResourceHandleInternal.h

    r138413 r139239  
    116116            , m_bodySize(0)
    117117            , m_bodyDataSent(0)
     118            , m_redirectCount(0)
    118119#endif
    119120#if PLATFORM(QT)
    … …  
    204205        RefPtr<NetworkingContext> m_context;
    205206        SoupSession* soupSession();
     207        int m_redirectCount;
    206208#endif
    207209#if PLATFORM(GTK)

  • trunk/Source/WebCore/platform/network/soup/ResourceError.h

    r138273 r139239  
    5151
    5252    static ResourceError httpError(SoupMessage*, GError*, SoupRequest*);
     53    static ResourceError transportError(SoupRequest*, int statusCode, const String& reasonPhrase);
    5354    static ResourceError genericIOError(GError*, SoupRequest*);
    5455    static ResourceError tlsError(SoupRequest*, unsigned tlsErrors, GTlsCertificate*);

  • trunk/Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp

    r138273 r139239  
    4949}
    5050
     51ResourceError ResourceError::transportError(SoupRequest* request, int statusCode, const String& reasonPhrase)
     52{
     53    return ResourceError(g_quark_to_string(SOUP_HTTP_ERROR), statusCode,
     54        failingURI(request), reasonPhrase);
     55}
     56
    5157ResourceError ResourceError::httpError(SoupMessage* message, GError* error, SoupRequest* request)
    5258{
    53     if (!message || !SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code))
     59    if (message && SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code))
     60        return transportError(request, message->status_code,
     61            String::fromUTF8(message->reason_phrase));
     62    else
    5463        return genericIOError(error, request);
    55     return ResourceError(g_quark_to_string(SOUP_HTTP_ERROR), message->status_code,
    56         failingURI(request), String::fromUTF8(message->reason_phrase));
    5764}
    5865

  • trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp

    r139062 r139239  
    223223};
    224224
    225 static void cleanupSoupRequestOperation(ResourceHandle*, bool isDestroying);
     225static bool createSoupRequestAndMessageForHandle(ResourceHandle*, const ResourceRequest&, bool isHTTPFamilyRequest);
     226static void cleanupSoupRequestOperation(ResourceHandle*, bool isDestroying = false);
    226227static void sendRequestCallback(GObject*, GAsyncResult*, gpointer);
    227228static void readCallback(GObject*, GAsyncResult*, gpointer);
    … …  
    331332
    332333    // The original response will be needed later to feed to willSendRequest in
    333     // restartedCallback() in case we are redirected. For this reason, so we store it
    334     // here.
     334    // doRedirect() in case we are redirected. For this reason, we store it here.
    335335    ResourceResponse response;
    336336    response.updateFromSoupMessage(message);
    … …  
    375375
    376376// Called each time the message is going to be sent again except the first time.
    377 // It's used mostly to let webkit know about redirects.
     377// This happens when libsoup handles HTTP authentication.
    378378static void restartedCallback(SoupMessage* message, gpointer data)
    379379{
    … …  
    385385        return;
    386386
     387#if ENABLE(WEB_TIMING)
    387388    ResourceResponse& redirectResponse = d->m_response;
    388 #if ENABLE(WEB_TIMING)
    389389    redirectResponse.setResourceLoadTiming(ResourceLoadTiming::create());
    390390    redirectResponse.resourceLoadTiming()->requestTime = monotonicallyIncreasingTime();
    391391#endif
    392 
    393     // WebCore only expects us to call willSendRequest when we are redirecting. soup
    394     // fires this signal also when it's handling authentication challenges, so in that
    395     // case we should not willSendRequest.
    396     if (isAuthenticationFailureStatusCode(redirectResponse.httpStatusCode()))
    397         return;
     392}
     393
     394static bool shouldRedirect(ResourceHandle* handle)
     395{
     396    ResourceHandleInternal* d = handle->getInternal();
     397    SoupMessage* message = d->m_soupMessage.get();
     398
     399    // Some 3xx status codes aren't actually redirects.
     400    if (message->status_code == 300 || message->status_code == 304 || message->status_code == 305 || message->status_code == 306)
     401        return false;
     402
     403    if (!soup_message_headers_get_one(message->response_headers, "Location"))
     404        return false;
     405
     406    return true;
     407}
     408
     409static bool shouldRedirectAsGET(SoupMessage* message, KURL& newURL, bool crossOrigin)
     410{
     411    if (message->method == SOUP_METHOD_GET)
     412        return false;
     413
     414    if (!newURL.protocolIsInHTTPFamily())
     415        return true;
     416
     417    switch (message->status_code) {
     418    case SOUP_STATUS_SEE_OTHER:
     419        return true;
     420    case SOUP_STATUS_FOUND:
     421    case SOUP_STATUS_MOVED_PERMANENTLY:
     422        if (message->method == SOUP_METHOD_POST)
     423            return true;
     424        break;
     425    }
     426
     427    if (crossOrigin && message->method == SOUP_METHOD_DELETE)
     428        return true;
     429
     430    return false;
     431}
     432
     433static void doRedirect(ResourceHandle* handle)
     434{
     435    ResourceHandleInternal* d = handle->getInternal();
     436    static const int maxRedirects = 20;
     437
     438    if (d->m_redirectCount++ > maxRedirects) {
     439        d->client()->didFail(handle, ResourceError::transportError(d->m_soupRequest.get(), SOUP_STATUS_TOO_MANY_REDIRECTS, "Too many redirects"));
     440        cleanupSoupRequestOperation(handle);
     441        return;
     442    }
    398443
    399444    ResourceRequest request = handle->firstRequest();
    400     request.setURL(KURL(handle->firstRequest().url(), soupURIToKURL(soup_message_get_uri(message))));
    401     request.setHTTPMethod(message->method);
     445    SoupMessage* message = d->m_soupMessage.get();
     446    const char* location = soup_message_headers_get_one(message->response_headers, "Location");
     447    KURL newURL = KURL(soupURIToKURL(soup_message_get_uri(message)), location);
     448    bool crossOrigin = !protocolHostAndPortAreEqual(request.url(), newURL);
     449    request.setURL(newURL);
     450
     451    if (shouldRedirectAsGET(message, newURL, crossOrigin)) {
     452        request.setHTTPMethod("GET");
     453        request.setHTTPBody(0);
     454        request.clearHTTPContentType();
     455    }
    402456
    403457    // Should not set Referer after a redirect from a secure resource to non-secure one.
    404     if (!request.url().protocolIs("https") && protocolIs(request.httpReferrer(), "https")) {
     458    if (!newURL.protocolIs("https") && protocolIs(request.httpReferrer(), "https"))
    405459        request.clearHTTPReferrer();
    406         soup_message_headers_remove(message->request_headers, "Referer");
    407     }
    408 
    409     const KURL& url = request.url();
    410     d->m_user = url.user();
    411     d->m_pass = url.pass();
     460
     461    d->m_user = newURL.user();
     462    d->m_pass = newURL.pass();
    412463    request.removeCredentials();
    413464
    414     if (!protocolHostAndPortAreEqual(request.url(), redirectResponse.url())) {
     465    if (crossOrigin) {
    415466        // If the network layer carries over authentication headers from the original request
    416467        // in a cross-origin redirect, we want to clear those headers here.
    417468        request.clearHTTPAuthorization();
    418         soup_message_headers_remove(message->request_headers, "Authorization");
    419469
    420470        // TODO: We are losing any username and password specified in the redirect URL, as this is the
    … …  
    423473        applyAuthenticationToRequest(handle, request, true);
    424474
    425     // Per-request authentication is handled via the URI-embedded username/password.
    426     GOwnPtr<SoupURI> newSoupURI(request.soupURI());
    427     soup_message_set_uri(message, newSoupURI.get());
     475    cleanupSoupRequestOperation(handle);
     476    if (!createSoupRequestAndMessageForHandle(handle, request, true)) {
     477        d->client()->cannotShowURL(handle);
     478        return;
     479    }
    428480
    429481    // If we sent credentials with this request's URL, we don't want the response to carry them to
    … …  
    431483    request.removeCredentials();
    432484
    433     if (d->client())
    434         d->client()->willSendRequest(handle, request, redirectResponse);
    435 
    436     if (d->m_cancelled)
    437         return;
    438 
    439     // Update the first party in case the base URL changed with the redirect
    440     String firstPartyString = request.firstPartyForCookies().string();
    441     if (!firstPartyString.isEmpty()) {
    442         GOwnPtr<SoupURI> firstParty(soup_uri_new(firstPartyString.utf8().data()));
    443         soup_message_set_first_party(d->m_soupMessage.get(), firstParty.get());
    444     }
     485    d->client()->willSendRequest(handle, request, d->m_response);
     486    handle->sendPendingRequest();
     487}
     488
     489static void redirectCloseCallback(GObject*, GAsyncResult* res, gpointer data)
     490{
     491    RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);
     492    ResourceHandleInternal* d = handle->getInternal();
     493
     494    g_input_stream_close_finish(d->m_inputStream.get(), res, 0);
     495    doRedirect(handle.get());
     496}
     497
     498static void redirectSkipCallback(GObject*, GAsyncResult* asyncResult, gpointer data)
     499{
     500    RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);
     501
     502    ResourceHandleInternal* d = handle->getInternal();
     503    ResourceHandleClient* client = handle->client();
     504
     505    if (d->m_cancelled || !client) {
     506        cleanupSoupRequestOperation(handle.get());
     507        return;
     508    }
     509
     510    GOwnPtr<GError> error;
     511    gssize bytesSkipped = g_input_stream_skip_finish(d->m_inputStream.get(), asyncResult, &error.outPtr());
     512    if (error) {
     513        client->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
     514        cleanupSoupRequestOperation(handle.get());
     515        return;
     516    }
     517
     518    if (bytesSkipped > 0) {
     519        g_input_stream_skip_async(d->m_inputStream.get(), G_MAXSSIZE, G_PRIORITY_DEFAULT,
     520            d->m_cancellable.get(), redirectSkipCallback, handle.get());
     521        return;
     522    }
     523
     524    g_input_stream_close_async(d->m_inputStream.get(), G_PRIORITY_DEFAULT, 0, redirectCloseCallback, handle.get());
    445525}
    446526
    … …  
    464544}
    465545
    466 static void cleanupSoupRequestOperation(ResourceHandle* handle, bool isDestroying = false)
     546static void cleanupSoupRequestOperation(ResourceHandle* handle, bool isDestroying)
    467547{
    468548    ResourceHandleInternal* d = handle->getInternal();
    … …  
    584664    }
    585665
    586     d->m_buffer = static_cast<char*>(g_slice_alloc(READ_BUFFER_SIZE));
    587 
    588666    if (soupMessage) {
     667        if (SOUP_STATUS_IS_REDIRECTION(soupMessage->status_code) && shouldRedirect(handle.get())) {
     668            d->m_inputStream = inputStream;
     669            g_input_stream_skip_async(d->m_inputStream.get(), G_MAXSSIZE, G_PRIORITY_DEFAULT,
     670                d->m_cancellable.get(), redirectSkipCallback, handle.get());
     671            return;
     672        }
     673
    589674        if (handle->shouldContentSniff() && soupMessage->status_code != SOUP_STATUS_NOT_MODIFIED) {
    590675            const char* sniffedType = soup_request_get_content_type(d->m_soupRequest.get());
    … …  
    612697        return;
    613698    }
     699
     700    d->m_buffer = static_cast<char*>(g_slice_alloc(READ_BUFFER_SIZE));
    614701
    615702    if (soupMessage && d->m_response.isMultipart()) {
    … …  
    880967    g_signal_connect(d->m_soupMessage.get(), "wrote-body-data", G_CALLBACK(wroteBodyDataCallback), handle);
    881968
     969    soup_message_set_flags(d->m_soupMessage.get(), static_cast<SoupMessageFlags>(soup_message_get_flags(d->m_soupMessage.get()) | SOUP_MESSAGE_NO_REDIRECT));
     970
    882971#if ENABLE(WEB_TIMING)
    883972    d->m_response.setResourceLoadTiming(ResourceLoadTiming::create());
    … …  
    889978}
    890979
    891 static bool createSoupRequestAndMessageForHandle(ResourceHandle* handle, bool isHTTPFamilyRequest)
     980static bool createSoupRequestAndMessageForHandle(ResourceHandle* handle, const ResourceRequest& request, bool isHTTPFamilyRequest)
    892981{
    893982    ResourceHandleInternal* d = handle->getInternal();
    … …  
    895984
    896985    GOwnPtr<GError> error;
    897     ResourceRequest& request = handle->firstRequest();
    898986
    899987    GOwnPtr<SoupURI> soupURI(request.soupURI());
    … …  
    9381026    applyAuthenticationToRequest(this, firstRequest(), false);
    9391027
    940     if (!createSoupRequestAndMessageForHandle(this, isHTTPFamilyRequest)) {
     1028    if (!createSoupRequestAndMessageForHandle(this, request, isHTTPFamilyRequest)) {
    9411029        this->scheduleFailure(InvalidURLFailure); // Error must not be reported immediately
    9421030        return true;
Note: See TracChangeset for help on using the changeset viewer.