Renderer sends cached metadata for data URL, which hits DCHECK in HttpUtil::SpecForRequest |
||||||
Issue descriptionChrome Version: 4235a8b857d6685b89ae1e5cc4ffe924a7a8ecf5 OS: Linux What steps will reproduce the problem? (1) Go to https://www.youtube.com/watch?v=sfuQVJ7xdbU (2) Say no to translate (3) Scroll down, comments start loading I printed out the |url.spec()| for what its worth: [150450:150504:0310/171142.172989:ERROR:http_util.cc(60)] data:application/javascript;base64,KGZ1bmN0aW9uKCkgewoJdmFyIHA7CgkvLyBodHRwczovL2RldmVsb3BlcnMuZ29vZ2xlLmNvbS9kb3VibGVjbGljay1ncHQvcmVmZXJlbmNlCgl2YXIgbm9vcGZuID0gZnVuY3Rpb24oKSB7CgkJOwoJfS5iaW5kKCk7Cgl2YXIgbm9vcHRoaXNmbiA9IGZ1bmN0aW9uKCkgewoJCXJldHVybiB0aGlzOwoJfTsKCXZhciBub29wbnVsbGZuID0gZnVuY3Rpb24oKSB7CgkJcmV0dXJuIG51bGw7Cgl9OwoJdmFyIG5vb3BhcnJheWZuID0gZnVuY3Rpb24oKSB7CgkJcmV0dXJuIFtdOwoJfTsKCXZhciBub29wc3RyZm4gPSBmdW5jdGlvbigpIHsKCQlyZXR1cm4gJyc7Cgl9OwoJLy8KCXZhciBjb21wYW5pb25BZHNTZXJ2aWNlID0gewoJCWFkZEV2ZW50TGlzdGVuZXI6IG5vb3B0aGlzZm4sCgkJZW5hYmxlU3luY0xvYWRpbmc6IG5vb3BmbiwKCQlzZXRSZWZyZXNoVW5maWxsZWRTbG90czogbm9vcGZuCgl9OwoJdmFyIGNvbnRlbnRTZXJ2aWNlID0gewoJCWFkZEV2ZW50TGlzdGVuZXI6IG5vb3B0aGlzZm4sCgkJc2V0Q29udGVudDogbm9vcGZuCgl9OwoJdmFyIFBhc3NiYWNrU2xvdCA9IGZ1bmN0aW9uKCkgewoJCTsKCX07CglwID0gUGFzc2JhY2tTbG90LnByb3RvdHlwZTsKCXAuZGlzcGxheSA9IG5vb3BmbjsKCXAuZ2V0ID0gbm9vcG51bGxmbjsKCXAuc2V0ID0gbm9vcHRoaXNmbjsKCXAuc2V0Q2xpY2tVcmwgPSBub29wdGhpc2ZuOwoJcC5zZXRUYWdGb3JDaGlsZERpcmVjdGVkVHJlYXRtZW50ID0gbm9vcHRoaXNmbjsKCXAuc2V0VGFyZ2V0aW5nID0gbm9vcHRoaXNmbjsKCXAudXBkYXRlVGFyZ2V0aW5nRnJvbU1hcCA9IG5vb3B0aGlzZm47Cgl2YXIgcHViQWRzU2VydmljZSA9IHsKCQlhZGRFdmVudExpc3RlbmVyOiBub29wdGhpc2ZuLAoJCWNsZWFyOiBub29wZm4sCgkJY2xlYXJDYXRlZ29yeUV4Y2x1c2lvbnM6IG5vb3B0aGlzZm4sCgkJY2xlYXJUYWdGb3JDaGlsZERpcmVjdGVkVHJlYXRtZW50OiBub29wdGhpc2ZuLAoJCWNsZWFyVGFyZ2V0aW5nOiBub29wdGhpc2ZuLAoJCWNvbGxhcHNlRW1wdHlEaXZzOiBub29wZm4sCgkJZGVmaW5lT3V0T2ZQYWdlUGFzc2JhY2s6IGZ1bmN0aW9uKCkgeyByZXR1cm4gbmV3IFBhc3NiYWNrU2xvdCgpOyB9LAoJCWRlZmluZVBhc3NiYWNrOiBmdW5jdGlvbigpIHsgcmV0dXJuIG5ldyBQYXNzYmFja1Nsb3QoKTsgfSwKCQlkaXNhYmxlSW5pdGlhbExvYWQ6IG5vb3BmbiwKCQlkaXNwbGF5OiBub29wZm4sCgkJZW5hYmxlQXN5bmNSZW5kZXJpbmc6IG5vb3BmbiwKCQllbmFibGVTaW5nbGVSZXF1ZXN0OiBub29wZm4sCgkJZW5hYmxlU3luY1JlbmRlcmluZzogbm9vcGZuLAoJCWVuYWJsZVZpZGVvQWRzOiBub29wZm4sCgkJZ2V0OiBub29wbnVsbGZuLAoJCWdldEF0dHJpYnV0ZUtleXM6IG5vb3BhcnJheWZuLAoJCXJlZnJlc2g6IG5vb3BmbiwKCQlzZXQ6IG5vb3B0aGlzZm4sCgkJc2V0Q2F0ZWdvcnlFeGNsdXNpb246IG5vb3B0aGlzZm4sCgkJc2V0Q2VudGVyaW5nOiBub29wZm4sCgkJc2V0Q29va2llT3B0aW9uczogbm9vcHRoaXNmbiwKCQlzZXRMb2NhdGlvbjogbm9vcHRoaXNmbiwKCQlzZXRQdWJsaXNoZXJQcm92aWRlZElkOiBub29wdGhpc2ZuLAoJCXNldFRhZ0ZvckNoaWxkRGlyZWN0ZWRUcmVhdG1lbnQ6IG5vb3B0aGlzZm4sCgkJc2V0VGFyZ2V0aW5nOiBub29wdGhpc2ZuLAoJCXNldFZpZGVvQ29udGVudDogbm9vcHRoaXNmbiwKCQl1cGRhdGVDb3JyZWxhdG9yOiBub29wZm4KCX07Cgl2YXIgU2l6ZU1hcHBpbmdCdWlsZGVyID0gZnVuY3Rpb24oKSB7CgkJOwoJfTsKCXAgPSBTaXplTWFwcGluZ0J1aWxkZXIucHJvdG90eXBlOwoJcC5hZGRTaXplID0gbm9vcHRoaXNmbjsKCXAuYnVpbGQgPSBub29wbnVsbGZuOwoJdmFyIFNsb3QgPSBmdW5jdGlvbigpIHsKCQk7Cgl9OwoJcCA9IFNsb3QucHJvdG90eXBlOwoJcC5hZGRTZXJ2aWNlID0gbm9vcHRoaXNmbjsKCXAuY2xlYXJDYXRlZ29yeUV4Y2x1c2lvbnMgPSBub29wdGhpc2ZuOwoJcC5jbGVhclRhcmdldGluZyA9IG5vb3B0aGlzZm47CglwLmRlZmluZVNpemVNYXBwaW5nID0gbm9vcHRoaXNmbjsKCXAuZ2V0ID0gbm9vcG51bGxmbjsKCXAuZ2V0QWRVbml0UGF0aCA9IG5vb3BhcnJheWZuOwoJcC5nZXRBdHRyaWJ1dGVLZXlzID0gbm9vcGFycmF5Zm47CglwLmdldENhdGVnb3J5RXhjbHVzaW9ucyA9IG5vb3BhcnJheWZuOwoJcC5nZXREb21JZCA9IG5vb3BzdHJmbjsKCXAuZ2V0U2xvdEVsZW1lbnRJZCA9IG5vb3BzdHJmbjsKCXAuZ2V0U2xvdElkID0gbm9vcHRoaXNmbjsKCXAuZ2V0VGFyZ2V0aW5nID0gbm9vcGFycmF5Zm47CglwLmdldFRhcmdldGluZ0tleXMgPSBub29wYXJyYXlmbjsKCXAuc2V0ID0gbm9vcHRoaXNmbjsKCXAuc2V0Q2F0ZWdvcnlFeGNsdXNpb24gPSBub29wdGhpc2ZuOwoJcC5zZXRDbGlja1VybCA9IG5vb3B0aGlzZm47CglwLnNldENvbGxhcHNlRW1wdHlEaXYgPSBub29wdGhpc2ZuOwoJcC5zZXRUYXJnZXRpbmcgPSBub29wdGhpc2ZuOwoJLy8KCXZhciBncHQgPSB3aW5kb3cuZ29vZ2xldGFnIHx8IHt9OwoJdmFyIGNtZCA9IGdwdC5jbWQgfHwgW107CglncHQuYXBpUmVhZHkgPSB0cnVlOwoJZ3B0LmNtZCA9IFtdOwoJZ3B0LmNtZC5wdXNoID0gZnVuY3Rpb24oYSkgewoJCXRyeSB7CgkJCWEoKTsKCQl9IGNhdGNoIChleCkgewoJCX0KCQlyZXR1cm4gMTsKCX07CglncHQuY29tcGFuaW9uQWRzID0gZnVuY3Rpb24oKSB7IHJldHVybiBjb21wYW5pb25BZHNTZXJ2aWNlOyB9OwoJZ3B0LmNvbnRlbnQgPSBmdW5jdGlvbigpIHsgcmV0dXJuIGNvbnRlbnRTZXJ2aWNlOyB9OwoJZ3B0LmRlZmluZU91dE9mUGFnZVNsb3QgPSBmdW5jdGlvbigpIHsgcmV0dXJuIG5ldyBTbG90KCk7IH07CglncHQuZGVmaW5lU2xvdCA9IGZ1bmN0aW9uKCkgeyByZXR1cm4gbmV3IFNsb3QoKTsgfTsKCWdwdC5kZXN0cm95U2xvdHMgPSBub29wZm47CglncHQuZGlzYWJsZVB1Ymxpc2hlckNvbnNvbGUgPSBub29wZm47CglncHQuZGlzcGxheSA9IG5vb3BmbjsKCWdwdC5lbmFibGVTZXJ2aWNlcyA9IG5vb3BmbjsKCWdwdC5nZXRWZXJzaW9uID0gbm9vcHN0cmZuOwoJZ3B0LnB1YmFkcyA9IGZ1bmN0aW9uKCkgeyByZXR1cm4gcHViQWRzU2VydmljZTsgfTsKCWdwdC5wdWJhZHNSZWFkeSA9IHRydWU7CglncHQuc2V0QWRJZnJhbWVUaXRsZSA9IG5vb3BmbjsKCWdwdC5zaXplTWFwcGluZyA9IGZ1bmN0aW9uKCkgeyByZXR1cm4gbmV3IFNpemVNYXBwaW5nQnVpbGRlcigpOyB9OwoJd2luZG93Lmdvb2dsZXRhZyA9IGdwdDsKCXdoaWxlICggY21kLmxlbmd0aCAhPT0gMCApIHsKCQlncHQuY21kLnB1c2goY21kLnNoaWZ0KCkpOwoJfQp9KSgpOw== [150450:150504:0310/171142.173118:FATAL:http_util.cc(62)] Check failed: url.is_valid() && (url.SchemeIsHTTPOrHTTPS() || url.SchemeIs("ftp") || url.SchemeIsWSOrWSS()). #0 0x7f9bf60e5bbb base::debug::StackTrace::StackTrace() #1 0x7f9bf60e424c base::debug::StackTrace::StackTrace() #2 0x7f9bf615273f logging::LogMessage::~LogMessage() #3 0x7f9bf51c8218 net::HttpUtil::SpecForRequest() #4 0x7f9bf5560308 net::HttpCache::GenerateCacheKey() #5 0x7f9bf5584c4e net::HttpCache::Transaction::DoGetBackendComplete() #6 0x7f9bf557fed2 net::HttpCache::Transaction::DoLoop() #7 0x7f9bf5581864 net::HttpCache::Transaction::Start() #8 0x7f9bf555eae2 net::HttpCache::MetadataWriter::Write() #9 0x7f9bf5560112 net::HttpCache::WriteMetadata() #10 0x7f9beffda66c content::RenderMessageFilter::OnCacheableMetadataAvailable() #11 0x7f9beffe1c62 _ZN4base20DispatchToMethodImplIPN7content19RenderMessageFilterEMS2_FvRK4GURLNS_4TimeERKNSt7__debug6vectorIcSaIcEEEERKSt5tupleIJS4_S7_SB_EEJLm0ELm1ELm2EEEEvRKT_T0_OT1_NS_13IndexSequenceIJXspT2_EEEE #12 0x7f9beffe1b80 _ZN4base16DispatchToMethodIPN7content19RenderMessageFilterEMS2_FvRK4GURLNS_4TimeERKNSt7__debug6vectorIcSaIcEEEERKSt5tupleIJS4_S7_SB_EEEEvRKT_T0_OT1_ #13 0x7f9beffe1aff _ZN3IPC16DispatchToMethodIN7content19RenderMessageFilterEMS2_FvRK4GURLN4base4TimeERKNSt7__debug6vectorIcSaIcEEEEvSt5tupleIJS3_S7_SB_EEEEvPT_T0_PT1_RKT2_ #14 0x7f9beffdc36f _ZN3IPC8MessageTI54RenderProcessHostMsg_DidGenerateCacheableMetadata_MetaSt5tupleIJ4GURLN4base4TimeENSt7__debug6vectorIcSaIcEEEEEvE8DispatchIN7content19RenderMessageFilterESE_vMSE_FvRKS3_S5_RKS9_EEEbPKNS_7MessageEPT_PT0_PT1_T2_ #15 0x7f9beffda178 content::RenderMessageFilter::OnMessageReceived()
,
Mar 10 2017
Looks like there are two bugs here: One in the deep dark heart of renderer process, which is asking for cacheable metadata for a data URL. One is browser land, where we pass an untrusted URL back to the network stack's cache layer, without validation. Neither of these is a network stack bug. I guess the cache storage team owns both chunks of code? Assigning this bug randomly to one of them.
,
Mar 17 2017
nhiroki@: Do you want to own this or should it be set to Untriaged for the Storage team to pick up? I'm not sure I see the connection to CacheStorage from the stack trace, onless OnCacheableMetadataAvailable is something specific to CacheStorage.
,
Mar 17 2017
I think we should check the URL schema in Resource::CachedMetadataHandlerImpl::sendToPlatform()
,
Jun 9 2017
This issue still exists, it would be great if this could be fixed or if at least the failing DCHECK is removed. It is currently impossible to visit some websites in a dev build like html5-demos.appspot.com
,
Jun 12 2017
A nonsense request is being sent to the cache, resulting in potentially undefined behavior. I don't think the solution, even short term, is to just remove the DCHECK.
,
Jun 12 2017
If crashing developers is helping resolve this then disabling the DCHECK until it's resolved would not be a good idea. Otherwise it's causing pain and has already done it's job to uncover this problem.
,
Jun 12 2017
[+jkarlin]: Who owns RenderMessageFilter::OnCacheableMetadataAvailable, do you know?
,
Jun 12 2017
This isn't cachestorage, if it were we'd see OnCacheableMetadataAvailableForCacheStorage in the call stack instead of OnCacheableMetadataAvailable. That said, we should verify that the URL is sane in both functions. I agree that removing the DCHECK isn't the answer. As best I can tell it's a loader issue so assigning to Blink>Loader. +cc vogelheim who is familiar with v8's reporting of cached code.
,
Jun 12 2017
Marking unconfirmed for loader triaging.
,
Jun 12 2017
Can we assign to a loader owner?
,
Jun 13 2017
We send cached data in blink::Resource::CachedMetadataHandlerImpl::SetCachedMetadata(), which is primarily called in V8ScriptRunner.cpp at following locations: CompileAndProduceCache() PostStreamCompile() SetCacheTimeStamp() Probably we shouldn't try caching compiled metadata for data: URL there, given that it wouldn't make much sense. vogelheim@- do you think you can fix this?
,
Jun 13 2017
#9: + #12: I'm not sure what to make of this. To cache anything, we need a CachedMetadatahandler. And we only create those if we have an http(s) resource: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp?l=288-290 While that particular bit has been refactored at some point (when code caching for ServiceWorkers was supported), the basic logic that we only attempt to cache for http(s)-URLs has been there from the beginning of the code cache implementation. So... I think what Kinuko intends at #12 is already there. I'll see if I can reproduce this...
,
Jun 13 2017
On tip-of-tree this page reproducably asserts for me, but somewhere completely different: [1:1:0613/111352.859875:FATAL:PrePaintTreeWalk.cpp(41)] Check failed: parent_context.tree_builder_context->is_actually_needed. #0 0x7f5a31cfc06b base::debug::StackTrace::StackTrace() #1 0x7f5a31cfad6c base::debug::StackTrace::StackTrace() #2 0x7f5a31d6e9f3 logging::LogMessage::~LogMessage() #3 0x7f5a1f06645c blink::PrePaintTreeWalkContext::PrePaintTreeWalkContext() #4 0x7f5a1f065d3d blink::PrePaintTreeWalk::Walk() #5 0x7f5a1f065e95 blink::PrePaintTreeWalk::Walk() #6 0x7f5a1f065e95 blink::PrePaintTreeWalk::Walk() #7 0x7f5a1f065e95 blink::PrePaintTreeWalk::Walk() #8 0x7f5a1f065e95 blink::PrePaintTreeWalk::Walk() #9 0x7f5a1f065e95 blink::PrePaintTreeWalk::Walk() (Maybe tip-of-tree is presently wonky w/ unrelated issues. I'll try again later.) |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by danakj@chromium.org
, Mar 10 2017Owner: rsleevi@chromium.org