Note: The crash works only when done through js.do, my best guess is that the way js.do frames the output has something to do with it.

Tested this issue on Windows-10 using chrome latest stable #57.0.2987.110 by following steps mentioned below.

1. Navigated to 57.0.2987.110
2. Switched to dev console and copy pasted the below code
   "webkitRequestFileSystem(0,9,o=>{o.root.getFile(1,{},f=>{f.createWriter(d=>
   {d.write(new Blob([1]));open('filesystem:'+location+'../temporary/1')})})})"
3. Clicked on Run and observed no crashes.

Reporter@ Could you please confirm is the above steps is the right way to reproduce this issue? 

Stack Trace:
-------------
Thread 13 CRASHED [0x0517a7ed / 0x00000000 @ 0x00007ffe6c4ccf93 ] MAGIC SIGNATURE THREAD
Stack Quality100%Show frame trust levels
0x00007ffe6c4ccf93	(chrome_elf.dll -crashpad_win.cc:155 )	DumpProcessWithoutCrash
0x00007ffe67fd46f2	(chrome.dll -bad_message.cc:53 )	content::bad_message::ReceivedBadMessage(int,content::bad_message::BadMessageReason)
0x00007ffe681f362c	(chrome.dll -render_process_host_impl.cc:3035 )	content::RenderProcessHostImpl::OnMojoError(int,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)
0x00007ffe68f74d5a	(chrome.dll -node_controller.cc:393 )	mojo::edk::NodeController::NotifyBadMessageFrom(mojo::edk::ports::NodeName const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)
0x00007ffe67e6df83	(chrome.dll -entrypoints.cc:254 )	MojoNotifyBadMessageImpl
0x00007ffe68660f69	(chrome.dll -validation_errors.cc:89 )	mojo::internal::ReportValidationError(mojo::internal::ValidationContext *,mojo::internal::ValidationError,char const *)
0x00007ffe6866103e	(chrome.dll -validation_errors.cc:105 )	mojo::internal::ReportValidationErrorForMessage(mojo::Message *,mojo::internal::ValidationError,char const *)
0x00007ffe67da4d7d	(chrome.dll -render_message_filter.mojom.cc:1202 )	content::mojom::RenderMessageFilterStubDispatch::AcceptWithResponder(content::mojom::RenderMessageFilter *,mojo::internal::SerializationContext *,mojo::Message *,mojo::MessageReceiverWithStatus *)
0x00007ffe68662a6c	(chrome.dll -interface_endpoint_client.cc:310 )	mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message *)
0x00007ffe686652e1	(chrome.dll -filter_chain.cc:40 )	mojo::FilterChain::Accept(mojo::Message *)
0x00007ffe68dd2813	(chrome.dll -ipc_mojo_bootstrap.cc:646 )	IPC::`anonymous namespace'::ChannelAssociatedGroupController::Accept
0x00007ffe686652e1	(chrome.dll -filter_chain.cc:40 )	mojo::FilterChain::Accept(mojo::Message *)
0x00007ffe68663f52	(chrome.dll -connector.cc:246 )	mojo::Connector::ReadSingleMessage(unsigned int *)
0x00007ffe68663bd2	(chrome.dll -connector.cc:205 )	mojo::Connector::OnHandleReadyInternal(unsigned int)
0x00007ffe68666cab	(chrome.dll -watcher.cc:83 )	mojo::Watcher::OnHandleReady(unsigned int)
0x00007ffe67f3f290	(chrome.dll -bind_internal.h:339 )	base::internal::Invoker<base::internal::BindState<void ( media::cast::UdpTransport::*)(int),base::WeakPtr<media::cast::UdpTransport>,net::Error>,void >::Run(base::internal::BindStateBase *)
0x00007ffe6806832f	(chrome.dll -callback.h:68 )	base::internal::RunMixin<base::Callback<void ,0,0> >::Run( ?? )
0x00007ffe6864dac1	(chrome.dll -task_annotator.cc:52 )	base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask *)
0x00007ffe685fb645	(chrome.dll -message_loop.cc:421 )	base::MessageLoop::RunTask(base::PendingTask *)
0x00007ffe685fc154	(chrome.dll -message_loop.cc:523 )	base::MessageLoop::DoWork()
0x00007ffe6864e826	(chrome.dll -message_pump_win.cc:475 )	base::MessagePumpForIO::DoRunLoop()
0x00007ffe6864dcf3	(chrome.dll -message_pump_win.cc:56 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x00007ffe68624c34	(chrome.dll -run_loop.cc:37 )	base::RunLoop::Run()
0x00007ffe67ffd72c	(chrome.dll -browser_thread_impl.cc:276 )	content::BrowserThreadImpl::IOThreadRun(base::RunLoop *)
0x00007ffe67ffd7df	(chrome.dll -browser_thread_impl.cc:311 )	content::BrowserThreadImpl::Run(base::RunLoop *)
0x00007ffe685fa3ac	(chrome.dll -thread.cc:328 )	base::Thread::ThreadMain()
0x00007ffe685cd33f	(chrome.dll -platform_thread_win.cc:84 )	base::`anonymous namespace'::ThreadFunc
0x00007ffe8c2e13d1	(KERNEL32.DLL + 0x000013d1 )	BaseThreadInitThunk
0x00007ffe8e7554e3	(ntdll.dll + 0x000154e3 )	RtlUserThreadStart

Not a Mojo bug but I'll investigate anyway. From crash keys:


"Received bad user message: Validation failed for RenderMessageFilter::CreateNewWindow deserializer [VALIDATION_ERROR_DESERIALIZATION_FAILED]"

so somehow this can cause the renderer to send a bad message. There aren't many ways this message can fail validation so it's kind of surprising.

A repro would be nice, unfortunately the steps from comment #1 do not repro for me on M57 or ToT.

Hrmm I cant seem to repro using incognito. Might have to do with an addon I have, Will investigate more.

This is only confusing me more. If I go to a saved js.do page (eg http://js.do/code/143440 ) the crash doesn't occur. The only thing that changed AFAICT is the URL. 

Will update if I get any other clues.

Ok my bad... This has nothing to do with filesystem api but probably the filesystem URI scheme.

I can still repro tab crash by copying only the following:

open('filesystem:'+location+'../temporary/1')

If I remove 'location' no crash occurs. 

I believe filesystem uri is not enabled in incognito, so I guess that solves that riddle. 

Also if I remove the '../' from the open function, no crash occurs.

Thanks, this is a good, reliable repro. Sounds like some extra filtering
needs to be done in the renderer before we try forwarding such requests to
the browser. If I get some time soon I will take a quick look and hopefully
find either a simple fix or a better owner.

Adding michaeln who may be able to help triage from a storage API perspective

The path navigation in the input is very unsafe

open ("filesystem:http://js.do/../temporary/1")

It's attempting to overwrite the host part of the http origin. We're right to reject that input, crashing the renderer may not the best way of going about it.

In dbg builds, the renderer hits a DCHECK and dies prior to sending a message to the browser to open the window.

void GURL::InitializeFromCanonicalSpec() {
  if (is_valid_ && SchemeIsFileSystem()) {
    inner_url_.reset(
        new GURL(spec_.data(), parsed_.Length(),
                 *parsed_.inner_parsed(), true));
  }

#ifndef NDEBUG
  // For testing purposes, check that the parsed canonical URL is identical to
  // what we would have produced. Skip checking for invalid URLs have no meaning
  // and we can't always canonicalize then reproducibly.
  if (is_valid_) {
    DCHECK(!spec_.empty());
    url::Component scheme;
    // We can't do this check on the inner_url of a filesystem URL, as
    // canonical_spec actually points to the start of the outer URL, so we'd
    // end up with infinite recursion in this constructor.
    if (!url::FindAndCompareScheme(spec_.data(), spec_.length(),
                                   url::kFileSystemScheme, &scheme) ||
        scheme.begin == parsed_.scheme.begin) {
      // We need to retain trailing whitespace on path URLs, as the |parsed_|
      // spec we originally received may legitimately contain trailing white-
      // space on the path or  components e.g. if the #ref has been
      // removed from a "foo:hello #ref" URL (see  http://crbug.com/291747 ).
      GURL test_url(spec_, RETAIN_TRAILING_PATH_WHITEPACE);

      DCHECK(test_url.is_valid_ == is_valid_);    <<===== Hits this DCHECK
      DCHECK(test_url.spec_ == spec_);    <<==== Also, the ".."s have been stripped out of the spec_
                                             the values are equal, but they both contain
                                             "filesystem:http://js.do//temporary/1"

      DCHECK(test_url.parsed_.scheme == parsed_.scheme);
      DCHECK(test_url.parsed_.username == parsed_.username);
      DCHECK(test_url.parsed_.password == parsed_.password);
      DCHECK(test_url.parsed_.host == parsed_.host);
      DCHECK(test_url.parsed_.port == parsed_.port);
      DCHECK(test_url.parsed_.path == parsed_.path);
      DCHECK(test_url.parsed_.query == parsed_.query);
      DCHECK(test_url.parsed_.ref == parsed_.ref);
    }
  }
#endif
}



>	url_lib.dll!GURL::InitializeFromCanonicalSpec() Line 161	C++
 	url_lib.dll!GURL::GURL(std::basic_string<char,std::char_traits<char>,std::allocator<char> > canonical_spec, const url::Parsed & parsed, bool is_valid) Line 116	C++
 	content.dll!blink::WebURL::operator GURL() Line 92	C++
 	content.dll!content::RenderViewImpl::CreateView(blink::WebLocalFrame * creator, const blink::WebURLRequest & request, const blink::WebWindowFeatures & features, const blink::WebString & frame_name, blink::WebNavigationPolicy policy, bool suppress_opener) Line 1419	C++
 	blink_web.dll!blink::ChromeClientImpl::CreateWindow(blink::LocalFrame * frame, const blink::FrameLoadRequest & r, const blink::WebWindowFeatures & features, blink::NavigationPolicy navigation_policy) Line 289	C++
 	blink_core.dll!blink::CreateNewWindow(blink::LocalFrame & opener_frame, const blink::FrameLoadRequest & request, const blink::WebWindowFeatures & features, blink::NavigationPolicy policy, bool & created) Line 295	C++
 	blink_core.dll!blink::CreateWindowHelper(blink::LocalFrame & opener_frame, blink::LocalFrame & active_frame, blink::LocalFrame & lookup_frame, const blink::FrameLoadRequest & request, const blink::WebWindowFeatures & features, blink::NavigationPolicy policy, bool & created) Line 390	C++
 	blink_core.dll!blink::CreateWindow(const WTF::String & url_string, const WTF::AtomicString & frame_name, const WTF::String & window_features_string, blink::LocalDOMWindow & calling_window, blink::LocalFrame & first_frame, blink::LocalFrame & opener_frame, blink::ExceptionState & exception_state) Line 445	C++
 	blink_core.dll!blink::LocalDOMWindow::open(const WTF::String & url_string, const WTF::AtomicString & frame_name, const WTF::String & window_features_string, blink::LocalDOMWindow * calling_window, blink::LocalDOMWindow * entered_window, blink::ExceptionState & exception_state) Line 1641	C++
 	blink_core.dll!blink::V8Window::openMethodCustom(const v8::FunctionCallbackInfo<v8::Value> & info) Line 279	C++
 	blink_core.dll!blink::V8Window::openMethodCallback(const v8::FunctionCallbackInfo<v8::Value> & info) Line 7686	C++

It dies in the browser process when deserializing the kFrameHost_CreateNewWindow_Name message, presumably because the target_url is invalid. Here's the message reported to ReportValidationErrorForMessage from within FrameHostStubDispatch::AcceptWithResponder...

error = "Received bad user message: Validation failed for FrameHost::CreateNewWindow deserializer [VALIDATION_ERROR_DESERIALIZATION_FAILED]"

I think the renderer could test if target_url.is_valid() prior to proceeding and bail out if not. A small change to RenderViewImpl::CreateView().

Hmmm... the problem is that target_url on the sending side claims to be valid (the GURL has accepted data from a WebURL which had claimed to be valid), but it deserializes to an invlalid GURL on the receiving side.

So the problem lies somewhere up the stack, closer to LocalDOMWindow::open(), a "valid" url with an invalid spec is being produced somehow.

Not a real fix, but a small change to RenderViewImpl::CreateView avoids the crash, reparse the target_url instead of directly assigning it, then test its validity prior to proceeding.

  if (!request.IsNull()) {
    params->referrer = GetReferrerFromRequest(creator, request);
    params->target_url = GURL(request.Url().GetString().Utf8());
    if (!params->target_url.is_valid())
      return nullptr;
  }

This is a GURL/KURL bug.

GURL a("filesystem:http://acme.com/../temporary/1") produces a url that is_valid()

but

GURL b(a.spec()) produces a url that is not valid.

Same problem for KURL.

The validity does not survive the round trip to a serialized format back to a URL object. The canonicalized format strips the ".."

spec == "filesystem:http://acme.com//temporary/1"

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot