New issue
Advanced search Search tips

Issue 846246 link

Starred by 2 users
Status: Fixed
Owner:
bashi@chromium.org
Closed: Jun 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

S13nServiceWorker: ServiceWorkerBrowserTest.ResponseFromHTTPServiceWorkerIsNotMarkedAsSecure is crashing

Project Member Reported by bashi@chromium.org, May 24 2018 
Stacktrace:

#0 0x7f0f7197ee8d base::debug::StackTrace::StackTrace()                                        
#1 0x7f0f716a94bc base::debug::StackTrace::StackTrace()
#2 0x0000024afff4 content::(anonymous namespace)::DumpStackTraceSignalHandler()
#3 0x7f0f55cdd030 <unknown>
#4 0x7f0f708f0f89 net::X509Certificate::EqualsExcludingChain()
#5 0x0000010bc594 content::ServiceWorkerBrowserTest_ResponseFromHTTPSServiceWorkerIsMarkedAsSecure_Test::RunTestOnMainThread()
#6 0x0000024afa13 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
...

The crash is caused by a null-dereference. entry->GetSSL().certificate is null.

We can fix this by requesting SSLInfo in ServiceWorkerNewScriptLoader. Here is why:
- NavigationEntry::GetSSL() returns SSLStatus which is initialized by a SSLInfo
- SSLInfo for SW intercepted navigation response is set by ServiceWorkerNavigationLoader
- ServiceWorkerNavigationLoader uses the service worker script's SSLInfo[1]
- That SSLInfo is set by ServiceWorkerNewScriptLoader when it receives a response from the network[2]

[1] https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_navigation_loader.cc?l=269&rcl=3d2d8991f69d3d05c4dad90b9314ecb8f11cb9ba
[2] https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_new_script_loader.cc?l=224&rcl=fca1f01a6a5eeeeee1131778c806273544487ade
Project Member

Comment 1 by bugdroid1@chromium.org, May 28 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/453e974148a6338b8a87b8dd61e416920dee37a8

commit 453e974148a6338b8a87b8dd61e416920dee37a8
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Mon May 28 09:18:13 2018

S13nSW: Ask network to set SSLInfo in ServiceWorkerNewScriptLoader

ServiceWorkerBrowserTest.ResponseFromHTTPSServiceWorkerIsMarkedAsSecure
is crashing because a NavigationEntry of SW-intercepted navigation
doesn't have a valid certificate. This CL fixes it by requesting
SSLInfo in ServiceWorkerNewScriptLoader. Here is why this fixes it:

- NavigationEntry::GetSSL() returns SSLStatus which is initialized
  by a SSLInfo.
- SSLInfo for SW intercepted navigation response is set by
  ServiceWorkerNavigationLoader.
- ServiceWorkerNavigationLoader uses the service worker script's
  SSLInfo.
- That SSLInfo is set by ServiceWorkerNewScriptLoader when it receives a
  response from the network.

Bug:  846246 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I74fe5f41d6e17262429bbe91fa3b3ececbc06f87
Reviewed-on: https://chromium-review.googlesource.com/1071494
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562216}
[modify] https://crrev.com/453e974148a6338b8a87b8dd61e416920dee37a8/content/browser/service_worker/service_worker_new_script_loader.cc
[modify] https://crrev.com/453e974148a6338b8a87b8dd61e416920dee37a8/content/browser/service_worker/service_worker_new_script_loader.h
[modify] https://crrev.com/453e974148a6338b8a87b8dd61e416920dee37a8/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Comment 2 by bashi@chromium.org, Jun 1 2018

Status: Fixed (was: Started)
►

Sign in to add a comment