Add checks before redirecting with NetworkSession
Created attachment 271449 [details] Patch
Comment on attachment 271449 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271449&action=review > Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:102 > + if (!request.url().protocolIs("https") && WebCore::protocolIs(request.httpReferrer(), "https") && m_shouldClearReferrerOnHTTPSToHTTPRedirect) You should probably put the fastest part of this check (checking m_shouldClearReferrerOnHTTPSToHTTPRedirect) first.
Comment on attachment 271449 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271449&action=review > Source/WebKit2/ChangeLog:20 > + 1. When using NSURLConnection, the last http method and the current http method are not always equal, and this > + check needs to change the current http method to match the last http method when being redirected with an http > + status code 307. When using NSURLSession, the current http method is already updated properly. Can you assert that it didn't change? > Source/WebKit2/NetworkProcess/NetworkDataTask.h:114 > + void willPerformHTTPRedirection(const WebCore::ResourceResponse, WebCore::ResourceRequest&&, RedirectCompletionHandler); Do you really mean to pass ResourceResponse by value? > Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:93 > + if (!equalLettersIgnoringASCIICase(m_lastHTTPMethod, "get") && body && !body->isEmpty()) If you reordered the operands in the if expression to check body && !body->isEmpty() first, we'd be able to sometimes skip the more expensive string comparison. > Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:117 > + Could you add a FIXME here for implementing the credential storage bits?
Comment on attachment 271449 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271449&action=review >> Source/WebKit2/NetworkProcess/NetworkDataTask.h:114 >> + void willPerformHTTPRedirection(const WebCore::ResourceResponse, WebCore::ResourceRequest&&, RedirectCompletionHandler); > > Do you really mean to pass ResourceResponse by value? No. Will fix. >> Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:117 >> + > > Could you add a FIXME here for implementing the credential storage bits? We don't want to use a CredentialStorage if we can get away with it.
Comment on attachment 271449 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271449&action=review >>> Source/WebKit2/NetworkProcess/NetworkDataTask.h:114 >>> + void willPerformHTTPRedirection(const WebCore::ResourceResponse, WebCore::ResourceRequest&&, RedirectCompletionHandler); >> >> Do you really mean to pass ResourceResponse by value? > > No. Will fix. You should still change it, but I guess in this case it doesn't actually matter. We're passing a NSHTTPURLResponse * into this function, so in either case we'll only construct one ResourceResponse (either a temporary that is bound to a const reference or the argument value).
Done. It's a reference now. http://trac.webkit.org/changeset/196656