Changeset 193830 in webkit
- Timestamp:
- Dec 9, 2015 6:52:46 AM (8 years ago)
- Location:
- trunk
- Files:
-
- 9 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebKit2/ChangeLog
r193827 r193830 1 2015-12-09 Mario Sanchez Prada <mario@endlessm.com> 2 3 [GTK] Crash in WebProcess when loading large content with custom URI schemes 4 https://bugs.webkit.org/show_bug.cgi?id=144262 5 6 Reviewed by Carlos Garcia Campos. 7 8 Properly handle scenarios where errors happen after reading the first 9 chunk of data coming from the GInputStream provided by the application. 10 11 * UIProcess/API/gtk/WebKitWebContextPrivate.h: 12 * UIProcess/API/gtk/WebKitWebContext.cpp: 13 (webkitWebContextIsLoadingCustomProtocol): New, checks whether a load 14 is still in progress, after the startLoading method has been called. 15 * UIProcess/API/gtk/WebKitURISchemeRequest.cpp: 16 (webkitURISchemeRequestReadCallback): Early return if the stream has been 17 cancelled on finish_error, so that we make sure we don't keep on reading 18 the GInputStream after that point. 19 (webkit_uri_scheme_request_finish_error): Don't send a didFailWithError 20 message to the Network process if the load is not longer in progress. 21 * Shared/Network/CustomProtocols/soup/CustomProtocolManagerImpl.cpp: 22 (WebKit::CustomProtocolManagerImpl::didFailWithError): Handle the case where 23 an error is notified from the UI process after the first chunk has been read. 24 (WebKit::CustomProtocolManagerImpl::didReceiveResponse): Handle the case where 25 data might no longer be available if an error happened even before this point. 26 * WebProcess/soup/WebKitSoupRequestInputStream.h: 27 * WebProcess/soup/WebKitSoupRequestInputStream.cpp: 28 (webkitSoupRequestInputStreamDidFailWithError): Notify the custom GInputStream 29 that we no longer want to keep reading data in chunks due to a specific error. 30 (webkitSoupRequestInputStreamReadAsync): Early finish the GTask with a specific 31 error whenever webkitSoupRequestInputStreamDidFailWithError() has been called. 32 1 33 2015-12-09 Ryuan Choi <ryuan.choi@navercorp.com> 2 34 -
trunk/Source/WebKit2/Shared/Network/CustomProtocols/soup/CustomProtocolManagerImpl.cpp
r185774 r193830 117 117 ASSERT(data); 118 118 119 GRefPtr<GTask> task = data->releaseTask(); 120 ASSERT(task.get()); 121 g_task_return_new_error(task.get(), g_quark_from_string(error.domain().utf8().data()), 122 error.errorCode(), "%s", error.localizedDescription().utf8().data()); 119 // Either we haven't started reading the stream yet, in which case we need to complete the 120 // task first, or we failed reading it and the task was already completed by didLoadData(). 121 ASSERT(!data->stream || !data->task); 122 123 if (!data->stream) { 124 GRefPtr<GTask> task = data->releaseTask(); 125 ASSERT(task.get()); 126 g_task_return_new_error(task.get(), g_quark_from_string(error.domain().utf8().data()), 127 error.errorCode(), "%s", error.localizedDescription().utf8().data()); 128 } else 129 webkitSoupRequestInputStreamDidFailWithError(WEBKIT_SOUP_REQUEST_INPUT_STREAM(data->stream.get()), error); 123 130 124 131 m_customProtocolMap.remove(customProtocolID); … … 171 178 { 172 179 WebSoupRequestAsyncData* data = m_customProtocolMap.get(customProtocolID); 173 ASSERT(data); 180 // The data might have been removed from the request map if an error happened even before this point. 181 if (!data) 182 return; 183 174 184 ASSERT(data->task); 175 185 -
trunk/Source/WebKit2/UIProcess/API/gtk/WebKitURISchemeRequest.cpp
r185502 r193830 166 166 return; 167 167 } 168 169 // Need to check the stream before proceeding as it can be cancelled if finish_error 170 // was previously call, which won't be detected by g_input_stream_read_finish(). 171 if (!request->priv->stream) 172 return; 168 173 169 174 WebKitURISchemeRequestPrivate* priv = request->priv; … … 231 236 232 237 WebKitURISchemeRequestPrivate* priv = request->priv; 233 238 if (!webkitWebContextIsLoadingCustomProtocol(priv->webContext, priv->requestID)) 239 return; 240 241 priv->stream = nullptr; 234 242 WebCore::ResourceError resourceError(g_quark_to_string(error->domain), toWebCoreError(error->code), priv->uri.data(), String::fromUTF8(error->message)); 235 243 priv->webRequestManager->didFailWithError(priv->requestID, resourceError); -
trunk/Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp
r193763 r193830 1295 1295 } 1296 1296 1297 bool webkitWebContextIsLoadingCustomProtocol(WebKitWebContext* context, uint64_t customProtocolID) 1298 { 1299 return context->priv->uriSchemeRequests.get(customProtocolID); 1300 } 1301 1297 1302 void webkitWebContextCreatePageForWebView(WebKitWebContext* context, WebKitWebView* webView, WebKitUserContentManager* userContentManager, WebKitWebView* relatedView) 1298 1303 { -
trunk/Source/WebKit2/UIProcess/API/gtk/WebKitWebContextPrivate.h
r177721 r193830 43 43 void webkitWebContextStopLoadingCustomProtocol(WebKitWebContext*, uint64_t customProtocolID); 44 44 void webkitWebContextDidFinishLoadingCustomProtocol(WebKitWebContext*, uint64_t customProtocolID); 45 bool webkitWebContextIsLoadingCustomProtocol(WebKitWebContext*, uint64_t customProtocolID); 45 46 void webkitWebContextCreatePageForWebView(WebKitWebContext*, WebKitWebView*, WebKitUserContentManager*, WebKitWebView*); 46 47 void webkitWebContextWebViewDestroyed(WebKitWebContext*, WebKitWebView*); -
trunk/Source/WebKit2/WebProcess/soup/WebKitSoupRequestInputStream.cpp
r188594 r193830 24 24 #include <wtf/Threading.h> 25 25 #include <wtf/glib/GRefPtr.h> 26 #include <wtf/glib/GUniquePtr.h> 26 27 27 28 struct AsyncReadData { … … 42 43 uint64_t bytesReceived; 43 44 uint64_t bytesRead; 45 46 GUniquePtr<GError> error; 44 47 45 48 Lock readLock; … … 90 93 if (!webkitSoupRequestInputStreamHasDataToRead(stream) && !webkitSoupRequestInputStreamIsWaitingForData(stream)) { 91 94 g_task_return_int(task.get(), 0); 95 return; 96 } 97 98 if (stream->priv->error.get()) { 99 g_task_return_error(task.get(), stream->priv->error.release()); 92 100 return; 93 101 } … … 164 172 } 165 173 174 void webkitSoupRequestInputStreamDidFailWithError(WebKitSoupRequestInputStream* stream, const WebCore::ResourceError& resourceError) 175 { 176 GUniquePtr<GError> error(g_error_new(g_quark_from_string(resourceError.domain().utf8().data()), resourceError.errorCode(), "%s", resourceError.localizedDescription().utf8().data())); 177 if (stream->priv->pendingAsyncRead) { 178 AsyncReadData* data = stream->priv->pendingAsyncRead.get(); 179 g_task_return_error(data->task.get(), error.release()); 180 } else { 181 stream->priv->contentLength = stream->priv->bytesReceived; 182 stream->priv->error = WTF::move(error); 183 } 184 } 185 166 186 bool webkitSoupRequestInputStreamFinished(WebKitSoupRequestInputStream* stream) 167 187 { -
trunk/Source/WebKit2/WebProcess/soup/WebKitSoupRequestInputStream.h
r165647 r193830 21 21 #define WebKitSoupRequestInputStream_h 22 22 23 #include <WebCore/ResourceError.h> 23 24 #include <gio/gio.h> 24 25 … … 49 50 GInputStream* webkitSoupRequestInputStreamNew(uint64_t contentLength); 50 51 void webkitSoupRequestInputStreamAddData(WebKitSoupRequestInputStream*, const void* data, size_t dataLength); 52 void webkitSoupRequestInputStreamDidFailWithError(WebKitSoupRequestInputStream*, const WebCore::ResourceError&); 51 53 bool webkitSoupRequestInputStreamFinished(WebKitSoupRequestInputStream*); 52 54 -
trunk/Tools/ChangeLog
r193827 r193830 1 2015-12-09 Mario Sanchez Prada <mario@endlessm.com> 2 3 [GTK] Crash in WebProcess when loading large content with custom URI schemes 4 https://bugs.webkit.org/show_bug.cgi?id=144262 5 6 Reviewed by Carlos Garcia Campos. 7 8 Added new unit test to check the additional scenarios we now 9 handle for custom URI schemes. 10 11 * TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp: 12 (generateHTMLContent): New helper function to generate big enough content. 13 (testWebContextURIScheme): New unit test. 14 1 15 2015-12-09 Ryuan Choi <ryuan.choi@navercorp.com> 2 16 -
trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp
r186225 r193830 27 27 #include <wtf/glib/GRefPtr.h> 28 28 #include <wtf/glib/GUniquePtr.h> 29 #include <wtf/text/StringBuilder.h> 29 30 #include <wtf/text/StringHash.h> 30 31 … … 198 199 static const char* errorDomain = "test"; 199 200 static const int errorCode = 10; 200 static const char* errorMessage = "Error message."; 201 202 static const char* genericErrorMessage = "Error message."; 203 static const char* beforeReceiveResponseErrorMessage = "Error before didReceiveResponse."; 204 static const char* afterInitialChunkErrorMessage = "Error after reading the initial chunk."; 201 205 202 206 class URISchemeTest: public LoadTrackingTest { … … 230 234 g_assert(webkit_uri_scheme_request_get_web_view(request) == test->m_webView); 231 235 232 GRefPtr<GInputStream> inputStream = adoptGRef(g_memory_input_stream_new());233 test->assertObjectIsDeletedWhenTestFinishes(G_OBJECT(inputStream.get()));234 235 236 const char* scheme = webkit_uri_scheme_request_get_scheme(request); 236 237 g_assert(scheme); 237 238 g_assert(test->m_handlersMap.contains(String::fromUTF8(scheme))); 238 239 240 const URISchemeHandler& handler = test->m_handlersMap.get(String::fromUTF8(scheme)); 241 242 GRefPtr<GInputStream> inputStream = adoptGRef(g_memory_input_stream_new()); 243 test->assertObjectIsDeletedWhenTestFinishes(G_OBJECT(inputStream.get())); 244 245 const gchar* requestPath = webkit_uri_scheme_request_get_path(request); 246 239 247 if (!g_strcmp0(scheme, "error")) { 240 GUniquePtr<GError> error(g_error_new_literal(g_quark_from_string(errorDomain), errorCode, errorMessage)); 241 webkit_uri_scheme_request_finish_error(request, error.get()); 248 if (!g_strcmp0(requestPath, "before-response")) { 249 GUniquePtr<GError> error(g_error_new_literal(g_quark_from_string(errorDomain), errorCode, beforeReceiveResponseErrorMessage)); 250 // We call finish() and then finish_error() to make sure that not even 251 // the didReceiveResponse message is processed at the time of failing. 252 webkit_uri_scheme_request_finish(request, G_INPUT_STREAM(inputStream.get()), handler.replyLength, handler.mimeType.data()); 253 webkit_uri_scheme_request_finish_error(request, error.get()); 254 } else if (!g_strcmp0(requestPath, "after-first-chunk")) { 255 g_memory_input_stream_add_data(G_MEMORY_INPUT_STREAM(inputStream.get()), handler.reply.data(), handler.reply.length(), 0); 256 webkit_uri_scheme_request_finish(request, inputStream.get(), handler.replyLength, handler.mimeType.data()); 257 // We need to wait until we reach the load-committed state before calling webkit_uri_scheme_request_finish_error(), 258 // so we rely on the test using finishOnCommittedAndWaitUntilLoadFinished() to actually call it from loadCommitted(). 259 } else { 260 GUniquePtr<GError> error(g_error_new_literal(g_quark_from_string(errorDomain), errorCode, genericErrorMessage)); 261 webkit_uri_scheme_request_finish_error(request, error.get()); 262 } 242 263 return; 243 264 } 244 265 245 const URISchemeHandler& handler = test->m_handlersMap.get(String::fromUTF8(scheme));246 247 266 if (!g_strcmp0(scheme, "echo")) { 248 char* replyHTML = g_strdup_printf(handler.reply.data(), webkit_uri_scheme_request_get_path(request));267 char* replyHTML = g_strdup_printf(handler.reply.data(), requestPath); 249 268 g_memory_input_stream_add_data(G_MEMORY_INPUT_STREAM(inputStream.get()), replyHTML, strlen(replyHTML), g_free); 250 269 } else if (!g_strcmp0(scheme, "closed")) … … 262 281 } 263 282 283 virtual void loadCommitted() override 284 { 285 if (m_finishOnCommitted) { 286 GUniquePtr<GError> error(g_error_new_literal(g_quark_from_string(errorDomain), errorCode, afterInitialChunkErrorMessage)); 287 webkit_uri_scheme_request_finish_error(m_uriSchemeRequest.get(), error.get()); 288 } 289 290 LoadTrackingTest::loadCommitted(); 291 } 292 293 void finishOnCommittedAndWaitUntilLoadFinished() 294 { 295 m_finishOnCommitted = true; 296 waitUntilLoadFinished(); 297 m_finishOnCommitted = false; 298 } 299 264 300 GRefPtr<WebKitURISchemeRequest> m_uriSchemeRequest; 265 301 HashMap<String, URISchemeHandler> m_handlersMap; 302 bool m_finishOnCommitted { false }; 266 303 }; 304 305 String generateHTMLContent(unsigned contentLength) 306 { 307 String baseString("abcdefghijklmnopqrstuvwxyz0123457890"); 308 unsigned baseLength = baseString.length(); 309 310 StringBuilder builder; 311 builder.append("<html><body>"); 312 313 if (contentLength <= baseLength) 314 builder.append(baseString, 0, contentLength); 315 else { 316 unsigned currentLength = 0; 317 while (currentLength < contentLength) { 318 if ((currentLength + baseLength) <= contentLength) 319 builder.append(baseString); 320 else 321 builder.append(baseString, 0, contentLength - currentLength); 322 323 // Account for the 12 characters of the '<html><body>' prefix. 324 currentLength = builder.length() - 12; 325 } 326 } 327 builder.append("</body></html>"); 328 329 return builder.toString(); 330 } 267 331 268 332 static void testWebContextURIScheme(URISchemeTest* test, gconstpointer) … … 308 372 g_assert(!test->m_loadEvents.contains(LoadTrackingTest::LoadFailed)); 309 373 310 test->registerURISchemeHandler("error", 0, 0, 0); 374 // Anything over 8192 bytes will get multiple calls to g_input_stream_read_async in 375 // WebKitURISchemeRequest when reading data, but we still need way more than that to 376 // ensure that we reach the load-committed state before failing, so we use an 8MB HTML. 377 String longHTMLContent = generateHTMLContent(8 * 1024 * 1024); 378 test->registerURISchemeHandler("error", longHTMLContent.utf8().data(), -1, "text/html"); 311 379 test->m_loadEvents.clear(); 312 380 test->loadURI("error:error"); … … 315 383 g_assert(test->m_loadFailed); 316 384 g_assert_error(test->m_error.get(), g_quark_from_string(errorDomain), errorCode); 317 g_assert_cmpstr(test->m_error->message, ==, errorMessage); 385 g_assert_cmpstr(test->m_error->message, ==, genericErrorMessage); 386 387 test->m_loadEvents.clear(); 388 test->loadURI("error:before-response"); 389 test->waitUntilLoadFinished(); 390 g_assert(test->m_loadEvents.contains(LoadTrackingTest::ProvisionalLoadFailed)); 391 g_assert(test->m_loadFailed); 392 g_assert_error(test->m_error.get(), g_quark_from_string(errorDomain), errorCode); 393 g_assert_cmpstr(test->m_error->message, ==, beforeReceiveResponseErrorMessage); 394 395 test->m_loadEvents.clear(); 396 test->loadURI("error:after-first-chunk"); 397 test->finishOnCommittedAndWaitUntilLoadFinished(); 398 g_assert(!test->m_loadEvents.contains(LoadTrackingTest::ProvisionalLoadFailed)); 399 g_assert(test->m_loadEvents.contains(LoadTrackingTest::LoadFailed)); 400 g_assert(test->m_loadFailed); 401 g_assert_error(test->m_error.get(), g_quark_from_string(errorDomain), errorCode); 402 g_assert_cmpstr(test->m_error->message, ==, afterInitialChunkErrorMessage); 318 403 319 404 test->registerURISchemeHandler("closed", 0, 0, 0);
Note: See TracChangeset
for help on using the changeset viewer.