New issue
Advanced search Search tips

Issue 920905 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 803774



Sign in to add a comment

document.referrer is always empty string after clicking a link of signed exchange

Project Member Reported by horo@chromium.org, Jan 11

Issue description

Chrome Version: 72.0.3626.53

What steps will reproduce the problem?
(1) Enable chrome://flags/#allow-sxg-certs-without-extension
(2) Go https://sxg-test.appspot.com/
(3) Click hello_ec.sxg
(4) Open DevTools console and check the value of document.referrer.

What is the expected result?
"https://sxg-test.appspot.com/"

What happens instead?
"" (empty string)

This is because CreateRedirectInfo() in signed_exchange_loader.cc doesn't set redirect_info.new_referrer correctly.
https://chromium.googlesource.com/chromium/src/+/62a4f43/content/browser/web_package/signed_exchange_loader.cc#41

According to the spec https://wicg.github.io/webpackage/loading.html#mp-http-fetch, signed exchange must be treated as same as 303 redirect.
So document.referrer must be set correctly.
 
Blocking: 803774
Cc: -horo@chromium.org
Owner: horo@chromium.org
Status: Assigned (was: Untriaged)
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 21 (2 days ago)

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

commit d0e9b098e5f320f919460df29a6110cfade58031
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Mon Jan 21 03:19:31 2019

Refactor RedirectInfo::ComputeRedirectInfo()

Currently HttpResponseHeaders argument is used only to get the Referrer-Policy
header.

I'm going to use this method while loading signed exchange. While handling a
synthesized redirection, I don't want create a new HttpResponseHeaders just for
passing the Referrer-Policy header.
So this CL removes HttpResponseHeaders argument and introduces
referrer_policy_header argument.

Bug: 920905
Change-Id: Ie3f0e71b4c99845da246261824de4c23564eefc4
Reviewed-on: https://chromium-review.googlesource.com/c/1420500
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624513}
[modify] https://crrev.com/d0e9b098e5f320f919460df29a6110cfade58031/content/browser/devtools/devtools_url_loader_interceptor.cc
[modify] https://crrev.com/d0e9b098e5f320f919460df29a6110cfade58031/content/common/service_worker/service_worker_loader_helpers.cc
[modify] https://crrev.com/d0e9b098e5f320f919460df29a6110cfade58031/headless/test/test_network_interceptor.cc
[modify] https://crrev.com/d0e9b098e5f320f919460df29a6110cfade58031/net/url_request/redirect_info.cc
[modify] https://crrev.com/d0e9b098e5f320f919460df29a6110cfade58031/net/url_request/redirect_info.h
[modify] https://crrev.com/d0e9b098e5f320f919460df29a6110cfade58031/net/url_request/redirect_info_unittest.cc
[modify] https://crrev.com/d0e9b098e5f320f919460df29a6110cfade58031/net/url_request/redirect_util.cc
[modify] https://crrev.com/d0e9b098e5f320f919460df29a6110cfade58031/net/url_request/redirect_util.h
[modify] https://crrev.com/d0e9b098e5f320f919460df29a6110cfade58031/net/url_request/url_request_job.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Yesterday (41 hours ago)

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

commit c91e24e59777bd2bec78e9201e3e0cc96fb477ab
Author: Thomas Tangl <tangltom@chromium.org>
Date: Mon Jan 21 12:59:28 2019

Revert "Refactor RedirectInfo::ComputeRedirectInfo()"

This reverts commit d0e9b098e5f320f919460df29a6110cfade58031.

Reason for revert: 
Findit found this as a very likely culprit for consistent
failures on ChromeOS.
Failing test: UkmBrowserTest.LogsTabId

Original change's description:
> Refactor RedirectInfo::ComputeRedirectInfo()
> 
> Currently HttpResponseHeaders argument is used only to get the Referrer-Policy
> header.
> 
> I'm going to use this method while loading signed exchange. While handling a
> synthesized redirection, I don't want create a new HttpResponseHeaders just for
> passing the Referrer-Policy header.
> So this CL removes HttpResponseHeaders argument and introduces
> referrer_policy_header argument.
> 
> Bug: 920905
> Change-Id: Ie3f0e71b4c99845da246261824de4c23564eefc4
> Reviewed-on: https://chromium-review.googlesource.com/c/1420500
> Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#624513}

TBR=horo@chromium.org,caseq@chromium.org,mmenke@chromium.org

Change-Id: I80bc4e3dda151e4c0715aa7d408b0319e4be35cf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 920905
Reviewed-on: https://chromium-review.googlesource.com/c/1424944
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624571}
[modify] https://crrev.com/c91e24e59777bd2bec78e9201e3e0cc96fb477ab/content/browser/devtools/devtools_url_loader_interceptor.cc
[modify] https://crrev.com/c91e24e59777bd2bec78e9201e3e0cc96fb477ab/content/common/service_worker/service_worker_loader_helpers.cc
[modify] https://crrev.com/c91e24e59777bd2bec78e9201e3e0cc96fb477ab/headless/test/test_network_interceptor.cc
[modify] https://crrev.com/c91e24e59777bd2bec78e9201e3e0cc96fb477ab/net/url_request/redirect_info.cc
[modify] https://crrev.com/c91e24e59777bd2bec78e9201e3e0cc96fb477ab/net/url_request/redirect_info.h
[modify] https://crrev.com/c91e24e59777bd2bec78e9201e3e0cc96fb477ab/net/url_request/redirect_info_unittest.cc
[modify] https://crrev.com/c91e24e59777bd2bec78e9201e3e0cc96fb477ab/net/url_request/redirect_util.cc
[modify] https://crrev.com/c91e24e59777bd2bec78e9201e3e0cc96fb477ab/net/url_request/redirect_util.h
[modify] https://crrev.com/c91e24e59777bd2bec78e9201e3e0cc96fb477ab/net/url_request/url_request_job.cc

Comment 7 by tangltom@chromium.org, Yesterday (41 hours ago)

Here's the failure of the builder:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-dbg/10384

horo@ please take a look!

Log:

[ RUN      ] UkmBrowserTest.LogsTabId/1
[1:1:0121/034756.857168:INFO:hugepage_text.cc(68)] Mlocking text pages failed: Cannot allocate memory (12)
[22314:22314:0121/034757.739481:WARNING:chrome_browser_main_chromeos.cc(547)] Running as stub user with profile dir: test-user
[22314:22314:0121/034758.456596:INFO:remote_commands_service.cc(38)] Fetching remote commands.
[22314:22314:0121/034758.459880:WARNING:remote_commands_service.cc(40)] Client is not registered.
[22314:22314:0121/034758.460615:INFO:remote_commands_invalidator.cc(32)] Initialize RemoteCommandsInvalidator.
[22314:22314:0121/034758.461027:INFO:remote_commands_invalidator.cc(57)] Starting RemoteCommandsInvalidator.
[22314:22314:0121/034758.462050:INFO:remote_commands_invalidator.cc(123)] RemoteCommandsInvalidator ReloadPolicyData.
[22314:22314:0121/034758.462403:INFO:remote_commands_invalidator.cc(167)] Unregister RemoteCommandsInvalidator.
[22314:22314:0121/034758.756860:WARNING:wallpaper_controller_client.cc(358)] Cannot get wallpaper files id in RemovePolicyWallpaper. This should never happen under normal circumstances.
[22314:22314:0121/034759.319282:WARNING:personal_data_manager.cc(487)] 0x251200b265a0 refresh is done, notifying PersonalDataChanged
[22314:22314:0121/034759.401476:WARNING:personal_data_manager.cc(487)] 0x251200c65020 refresh is done, notifying PersonalDataChanged
[22314:22314:0121/034759.933475:ERROR:gpu_interface_provider.cc(87)] Not implemented reached in virtual void content::GpuInterfaceProvider::RegisterOzoneGpuInterfaces(service_manager::BinderRegistry *)
[22314:22314:0121/034800.595970:WARNING:loopback_server.cc(719)] Loopback sync persistent state file does not exist.
[22314:22314:0121/034800.598749:ERROR:http_bridge.cc(126)] Not implemented reached in virtual void syncer::HttpBridgeFactory::OnSignalReceived()
[22314:22491:0121/034800.669645:WARNING:simple_synchronous_entry.cc(1430)] Could not open platform files for entry.
[22314:22570:0121/034800.693656:WARNING:sync_encryption_handler_impl.cc(1080)] Nigori had empty encryption keybag.
[22314:22570:0121/034800.698315:WARNING:sync_encryption_handler_impl.cc(1080)] Nigori had empty encryption keybag.
[22314:22314:0121/034800.734362:ERROR:chrome_device_id_helper.cc(42)] Device ID is not set for user.
[22314:22314:0121/034800.734968:INFO:profile_sync_service.cc(1953)] ConfigureDataTypeManager not invoked because datatypes cannot be configured now
[22314:22314:0121/034800.750955:INFO:profile_sync_service.cc(1953)] ConfigureDataTypeManager not invoked because datatypes cannot be configured now
[22314:22314:0121/034800.812293:ERROR:account_tracker.cc(263)] OnOAuthError
[22314:22314:0121/034800.812348:WARNING:account_tracker.cc(189)] Failed to get UserInfo for stub-user@example.com
[22314:22314:0121/034800.930277:WARNING:personal_data_manager.cc(495)] 0x251200b265a0 has synced new data, refreshing
[22314:22314:0121/034801.231952:WARNING:personal_data_manager.cc(487)] 0x251200b265a0 refresh is done, notifying PersonalDataChanged
[22314:22621:0121/034802.004922:WARNING:embedded_test_server.cc(237)] Request not handled. Returning 404: /favicon.ico
[22314:22484:0121/034802.164731:WARNING:spdy_session.cc(3180)] Received HEADERS for invalid stream 107
[22314:22484:0121/034802.327757:WARNING:spdy_session.cc(3180)] Received HEADERS for invalid stream 155
[22314:22484:0121/034802.361175:WARNING:spdy_session.cc(3180)] Received HEADERS for invalid stream 151
[22314:22484:0121/034802.364625:WARNING:spdy_session.cc(3180)] Received HEADERS for invalid stream 145
[22314:22484:0121/034802.364874:WARNING:spdy_session.cc(3180)] Received HEADERS for invalid stream 143
[22314:22484:0121/034802.365129:WARNING:spdy_session.cc(3180)] Received HEADERS for invalid stream 129
[22314:22314:0121/034803.960649:FATAL:logo_service_impl.cc(596)] Check failed: !encoded_logo.
#0 0x7faed4290ab9 base::debug::StackTrace::StackTrace()
#1 0x7faed4095985 base::debug::StackTrace::StackTrace()
#2 0x7faed40d4b77 logging::LogMessage::~LogMessage()
#3 0x55816513a478 search_provider_logos::LogoServiceImpl::OnFreshLogoAvailable()
#4 0x558165139a3e search_provider_logos::LogoServiceImpl::OnFreshLogoParsed()
#5 0x558165143cfa base::internal::FunctorTraits<>::Invoke<>()
#6 0x558165143c19 base::internal::InvokeHelper<>::MakeItSo<>()
#7 0x558165143b69 _ZN4base8internal7InvokerINS0_9BindStateIMN21search_provider_logos15LogoServiceImplEFvPbbNSt4__Cr10unique_ptrINS3_11EncodedLogoENS6_14default_deleteIS8_EEEEEJNS_7WeakPtrIS4_EENS0_12OwnedWrapperIbEEbEEEFvSB_EE7RunImplISD_NS6_5tupleIJSF_SH_bEEEJLm0ELm1ELm2EEEEvOT_OT0_NS6_16integer_sequenceImJXspT1_EEEEOSB_
#8 0x558165143a29 base::internal::Invoker<>::RunOnce()
#9 0x55815c56ed73 _ZNO4base12OnceCallbackIFvNSt4__Cr6vectorIN4mojo9StructPtrIN6device5mojom13UsbDeviceInfoEEENS1_9allocatorIS8_EEEEEE3RunESB_
#10 0x5581651406b7 base::internal::ReplyAdapter<>()
#11 0x55815c5aaa9f base::internal::FunctorTraits<>::Invoke<>()
#12 0x558165140cf7 base::internal::InvokeHelper<>::MakeItSo<>()
#13 0x558165140ca8 _ZN4base8internal7InvokerINS0_9BindStateIPFvNS_12OnceCallbackIFvNSt4__Cr10unique_ptrIN21search_provider_logos11EncodedLogoENS4_14default_deleteIS7_EEEEEEEPNS5_ISA_NS8_ISA_EEEEEJSC_NS0_12OwnedWrapperISE_EEEEEFvvEE7RunImplISH_NS4_5tupleIJSC_SJ_EEEJLm0ELm1EEEEvOT_OT0_NS4_16integer_sequenceImJXspT1_EEEE
#14 0x558165140bae base::internal::Invoker<>::RunOnce()
#15 0x7faed406a84e _ZNO4base12OnceCallbackIFvvEE3RunEv
#16 0x7faed4214da1 base::(anonymous namespace)::PostTaskAndReplyRelay::RunReply()
#17 0x7faed421555f base::internal::FunctorTraits<>::Invoke<>()
#18 0x7faed42154b2 base::internal::InvokeHelper<>::MakeItSo<>()
#19 0x7faed4215452 _ZN4base8internal7InvokerINS0_9BindStateIPFvNS_12_GLOBAL__N_121PostTaskAndReplyRelayEEJS4_EEEFvvEE7RunImplIS6_NSt4__Cr5tupleIJS4_EEEJLm0EEEEvOT_OT0_NSB_16integer_sequenceImJXspT1_EEEE
#20 0x7faed421537e base::internal::Invoker<>::RunOnce()
#21 0x7faed406a84e _ZNO4base12OnceCallbackIFvvEE3RunEv
#22 0x7faed4096a85 base::debug::TaskAnnotator::RunTask()
#23 0x7faed40f228f base::MessageLoopImpl::RunTask()
#24 0x7faed40f2555 base::MessageLoopImpl::DeferOrRunPendingTask()
#25 0x7faed40f2ce7 base::MessageLoopImpl::DoWork()
#26 0x7faed42cc21a base::MessagePumpLibevent::Run()
#27 0x7faed40f1be6 base::MessageLoopImpl::Run()
#28 0x7faed415c37e base::RunLoop::Run()
#29 0x55816206678e content::WindowedNotificationObserver::Wait()
#30 0x558161ffebe8 content::WaitForLoadStopWithoutSuccessCheck()
#31 0x558161ffe765 content::WaitForLoadStop()
#32 0x55816130abc8 ui_test_utils::NavigateToURLWithDispositionBlockUntilNavigationsComplete()
#33 0x55816130ab5f ui_test_utils::NavigateToURLWithDisposition()
#34 0x55816130ab27 ui_test_utils::NavigateToURL()
#35 0x55815c7a2366 metrics::UkmBrowserTestBase::NavigateAndGetSource()
#36 0x55815c79d349 metrics::UkmBrowserTest_LogsTabId_Test::RunTestOnMainThread()
#37 0x558161ffab74 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#38 0x55815c39290d base::internal::FunctorTraits<>::Invoke<>()
#39 0x55815c392851 base::internal::InvokeHelper<>::MakeItSo<>()
#40 0x558161ffccc7 _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserTestBaseEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunImplIRKS6_RKNSt4__Cr5tupleIJS8_EEEJLm0EEEEvOT_OT0_NSF_16integer_sequenceImJXspT1_EEEE
#41 0x558161ffcbfc base::internal::Invoker<>::Run()
#42 0x55815c38549d _ZNKR4base17RepeatingCallbackIFvvEE3RunEv
#43 0x5581613cae9f ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#44 0x5581613c9c4e ChromeBrowserMainParts::PreMainMessageLoopRun()
#45 0x55815e189fd4 chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
#46 0x7faecaa23077 content::BrowserMainLoop::PreMainMessageLoopRun()
#47 0x7faec9ba45ed base::internal::FunctorTraits<>::Invoke<>()
#48 0x7faec9ba4531 base::internal::InvokeHelper<>::MakeItSo<>()
#49 0x7faecaa29537 _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserMainLoopEFivEJNS0_17UnretainedWrapperIS4_EEEEEFivEE7RunImplIRKS6_RKNSt4__Cr5tupleIJS8_EEEJLm0EEEEiOT_OT0_NSF_16integer_sequenceImJXspT1_EEEE
#50 0x7faecaa2946c base::internal::Invoker<>::Run()
#51 0x7faec9afd57d _ZNKR4base17RepeatingCallbackIFvvEE3RunEv
#52 0x7faecb5a511c content::StartupTaskRunner::RunAllTasksNow()
#53 0x7faecaa218a5 content::BrowserMainLoop::CreateStartupTasks()
#54 0x7faecaa2b91e content::BrowserMainRunnerImpl::Initialize()
#55 0x7faecaa1eb21 content::BrowserMain()
#56 0x7faecc48293b content::RunBrowserProcessMain()
#57 0x7faecc483bf6 content::ContentMainRunnerImpl::RunServiceManager()
#58 0x7faecc4836b0 content::ContentMainRunnerImpl::Run()
#59 0x7faecc480c79 content::ContentServiceManagerMainDelegate::RunEmbedderProcess()
#60 0x7faea6216d23 service_manager::Main()
#61 0x7faecc482445 content::ContentMain()
#62 0x558161ffa689 content::BrowserTestBase::SetUp()
#63 0x5581613034ae InProcessBrowserTest::SetUp()
#64 0x55816215151a SyncTest::SetUp()
#65 0x55815c7a37ee metrics::UkmBrowserTestBase::SetUp()

Comment 8 by horo@chromium.org, Yesterday (40 hours ago)

I'm not 100% sure, but I think this patch which landed just after my patch is more relevant to this crash of UkmBrowserTest.LogsTabId.
f207ec9 Adds a separator between custom tab bar and title, based on UX feedback.
https://chromium-review.googlesource.com/c/chromium/src/+/1414535

Project Member

Comment 9 by bugdroid1@chromium.org, Yesterday (36 hours ago)

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

commit faaa279bbc2e1656811684ffac781e8418a665a0
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Mon Jan 21 17:57:28 2019

Reland "Refactor RedirectInfo::ComputeRedirectInfo()"

This is a reland of d0e9b098e5f320f919460df29a6110cfade58031
Looks like this wasn't the culprit.

Original change's description:
> Refactor RedirectInfo::ComputeRedirectInfo()
>
> Currently HttpResponseHeaders argument is used only to get the Referrer-Policy
> header.
>
> I'm going to use this method while loading signed exchange. While handling a
> synthesized redirection, I don't want create a new HttpResponseHeaders just for
> passing the Referrer-Policy header.
> So this CL removes HttpResponseHeaders argument and introduces
> referrer_policy_header argument.
>
> Bug: 920905
> Change-Id: Ie3f0e71b4c99845da246261824de4c23564eefc4
> Reviewed-on: https://chromium-review.googlesource.com/c/1420500
> Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#624513}

TBR=horo@chromium.org,caseq@chromium.org,mmenke@chromium.org

Bug: 920905,  923894 
Change-Id: I0ad4c1ade7dbd47e257ed0d5a882c7ccd796e75d
Reviewed-on: https://chromium-review.googlesource.com/c/1425498
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624606}
[modify] https://crrev.com/faaa279bbc2e1656811684ffac781e8418a665a0/content/browser/devtools/devtools_url_loader_interceptor.cc
[modify] https://crrev.com/faaa279bbc2e1656811684ffac781e8418a665a0/content/common/service_worker/service_worker_loader_helpers.cc
[modify] https://crrev.com/faaa279bbc2e1656811684ffac781e8418a665a0/headless/test/test_network_interceptor.cc
[modify] https://crrev.com/faaa279bbc2e1656811684ffac781e8418a665a0/net/url_request/redirect_info.cc
[modify] https://crrev.com/faaa279bbc2e1656811684ffac781e8418a665a0/net/url_request/redirect_info.h
[modify] https://crrev.com/faaa279bbc2e1656811684ffac781e8418a665a0/net/url_request/redirect_info_unittest.cc
[modify] https://crrev.com/faaa279bbc2e1656811684ffac781e8418a665a0/net/url_request/redirect_util.cc
[modify] https://crrev.com/faaa279bbc2e1656811684ffac781e8418a665a0/net/url_request/redirect_util.h
[modify] https://crrev.com/faaa279bbc2e1656811684ffac781e8418a665a0/net/url_request/url_request_job.cc

Sign in to add a comment