[webkit-reviews] review denied: [Bug 132384] [GTK] unit test for tls redirection failure required : [Attachment 230573] WIP patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat May 3 04:45:32 PDT 2014
Carlos Garcia Campos <cgarcia at igalia.com> has denied review:
Bug 132384: [GTK] unit test for tls redirection failure required
https://bugs.webkit.org/show_bug.cgi?id=132384
Attachment 230573: WIP patch
https://bugs.webkit.org/attachment.cgi?id=230573&action=review
------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=230573&action=review
I think this test is actually showing that your patch was wrong, since we are
failing with TLS errors for a non HTTPS connection.
> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp:222
> + // Load a page with an invalid certificate that redirects to http.
> + test->loadURI(kHttpsServer->getURIForPath("/redirect-to-http").data());
> + test->waitUntilLoadFinished();
Shouldn't this fail?, the comment says it's a page with an invalid certificate.
> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp:226
> + // Now load a page with http and expect that the previous TLS failure
should cause the load to fail.
> + test->loadURI(kHttpsServer->getURIForPath("/index.html").data());
> + test->waitUntilLoadFailedWithTLSErrors();
I don't think this is correct. The errors of a previous load shouldn't affect
new loads. The problem is supposed to happen with redirections, not with two
independent loads.
> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp:252
> + } else if (g_str_equal(path, "/redirect-to-http/")) {
> + soup_message_set_redirect(message, SOUP_STATUS_MOVED_PERMANENTLY,
"");
I don't understand this either, this should redirect to another location, you
should use a redirect uri instead of "".
More information about the webkit-reviews
mailing list