New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 651895 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocking:
issue 594215
issue 639467
issue 649208


Participants' hotlists:
EnamelAndFriendsFixIt


Sign in to add a comment

Pluming through navigation initiator's origin and url from blink to NavigationHandle

Project Member Reported by jialiul@chromium.org, Sep 30 2016

Issue description

Per our discussion, we want to know origin and url of the true navigation initiator, preferably make them available in NavigationHandle. This information will help us accurately identify the initiator where navigation is targeting another already exiting frames/tab.


 
Description: Show this description
Labels: -Restrict-View-Google
Blocking: 639467
Labels: SafeBrowsing-Triaged
Owner: dcheng@chromium.org
Status: Assigned (was: Available)
Assigning to dcheng for first step.
Cc: mea...@chromium.org

Comment 6 by mea...@chromium.org, Oct 11 2016

Just so it helps with prioritization: Deprecation of data: navigations is also blocked on getting this information. Intervention owners are asking if there is an estimated timeline for this work :)

Blocking: 649208
dcheng@: Any updates here? 
Blocking: 652381
Labels: -Pri-2 OS-All Pri-1
This should be P1 since it's blocking two P1's now.
Cc: mkwst@chromium.org
Status: Started (was: Assigned)
Sorry, forgot to update the status on this bug.

I sent out an initial CL and talked with mkst@ 2 weeks ago to figure out the right plumbing for this (and to make sure it's not too redundant with what we already sort-of have). The current plan is to expose WebDocument as the requestingDocument on WebURLRequest. In didCommitProvisionalLoad(), we'll pull the original WebURLRequest and send the requesting origin and URL up to the browser then.

WRT timeline, I'm currently finishing off another CL, but this is my priority for this week.
Awesome, thanks for the update!
It seems crrev.com/2416523002 is ready to land. Is there anything still blocking this CL? 
Unfortunately, it doesn't work well with OOPIF, so we need to do this tracking entirely browser-side... but then we need to worry about potential races. This is something I'll be chatting with nasko@ about to figure out a good resolution. Sorry for the delay.
Hey @dcheng, any update? This change is critical so that we can block navigations to data: URIs. Please let us know if we can help unblock! 
Sorry for the delay! So just for documentation on the bug:

At a high-level, we want to be able to track this on the NavigationHandle for a request.

To do that, I originally wrote a CL to expose this as a Blink public API. The idea was that when a load commits, the renderer can pull the initiator URL out of the WebDataSource and send it up in an IPC to the browser process. Unfortunately, this doesn't work well for things like OOPIF: if a frame swaps processes, then the process that initiated the URL has to know to send the initiator URL up to the browser when it's swapped out... but because it's in a separate process, other data races are possible (the initiating frame could have been navigated elsewhere, for example).

Another possibility is to always send the initiator URL when creating the NavigationHandle in the browser process, but that feels heavyweight and requires trusting the renderer.

Instead, we'd like to just have NavigationHandle record the initiator's last committed URL when a NavigationHandle is created. This is a bit heavyweight (similar to how always sending the initiator URL from the renderer is), but it's necessary to avoid other sorts of races. However, I believe there are other races will prevent the initiator from being recorded correctly:
1. RenderFrame |initiator| initiates a navigation.
2. Before the browser process receives the navigation IPC, it sends an IPC to the renderer to swap out / detach |initiator|.
3. Browser process tries to lookup the initiator RFH, but it's gone (if it's a swap, I think it only stays in the pending delete list for 1 second; if it's a detach or the window is closed, it's removed immediately). Thus, we fail to record the initiator information.

It's not clear to me how easy it is for a renderer to manipulate IPC timing to trigger this case...

Finally, in the future, we believe we can make this lighterweight: creis@ is thinking about making RFH hold a pointer to the current FrameNavigationEntry (which would replace the last committed URL), making it lighterweight to record the initiator info. However, this still does not solve the IPC race issue...
OK, after discussing this with alexmos, creis, and nasko, we're comfortable with just dropping the navigation IPC if the initiator lookup fails. That solves the potential IPC races =)
I ran into this once again while implementing the blocking of data URLs. The blocking is done with a NavigationThrottle, and in order to support data to data navigations (e.g. links in data URLs) we need to know the initiator of the navigation.

Looks like https://codereview.chromium.org/2416523002/ got the lgtm, any updates since then? :)
Blocking: -652381 594215
Blocking: 632514
Cc: japhet@chromium.org
OK, so I'm finding that there's no satisfactory approach for implementing this. I've considered several approaches so far.

Solution #1: https://codereview.chromium.org/2416523002/: plumb the info out from the renderer through WebDataSource. Originally, I intended this data to only be read at commit time: however, this is difficult due to how OOPIF works... if we force a process swap, it's likely that we can lose this data.

Solution #2: https://codereview.chromium.org/2781723007/: this was the Blink side of the alternate approach to plumb the info out when beginning a navigation. The idea is once we know the initiator frame, we can send the frame routing ID up to the browser, and the browser uses this routing ID to fill in the origin/url of the initiator frame on the NavigationHandle. We're not trusting the renderer so send us the correct info, so yay. 

At least two problems...
- navigations are typically scheduled. So if a frame schedules a navigation on another frame, and then detaches itself, we'll lose the initiator frame info (as Document::frame() is null when the Document is detached).
- in case of IPC races to the browser (e.g. the initiator RFH gets swapped out while we're still sending the IPC), it's possible that we'll no longer be able to look up the initiator RFH.
- probably others

Solution #3: plumb navigations via their initiator frame. This is a large rewrite of how navigation works, and is likely impractical at the moment, since PlzNavigate has not yet shipped. The basic idea is that any navigation requests would be plumbed out via the initiator's WebFrameClient, along with the target frame... but this would make FrameLoader fraught with peril (since you must very carefully distinguish between |this| and |targetFrame|).

There may be web compat issues as well: it's not clear if there's web content that expects to fire off navigations that still work after the source Document is detached.

Solution #4: Use solution #1, but just always pass the info up. Non-ideal given that we have to trust the renderer again...
Thanks for the update!

It sounds like solution #2 is problematic because it tracks RFHs that can be detached. In my case (data URL blocking), I'm interested in knowing the top frame URL of the opener at the time the navigation was scheduled, mainly for window.open cases. Would it be possible to plumb out only the URL instead of the frame routing id? Does that solve the problem with the frame detaching itself, or having to look up the RFH? (obviously this only helps with my use case and not necessarily others :)
I talked with Nasko to get content OWNERS signoff.

There is no feasible way to implement a trustworthy initiator today: due to various races, we can't just plumb through a frame routing ID and hope to extract the correct information on the browser side.

Instead, I'll land a version of the original CL to expose this information from Blink, and we'll pass the origin/URL up in RenderFrameImpl::didStartProvisionalLoad() / RenderFrameProxy::navigate(). It'll be bound to the NavigationHandle when it's created, and will be marked 'untrustworthy' (since the information comes from the renderer), but it should be sufficient to satisfy the current use cases we have.

It's expected that we might be missing initiator information in certain contexts, but we can iterate from there and see what's still missing.
Would the origin/URL be of the initiator frame? If so, would it be possible to also pass through the top frame URL of the initiator? 

In my use case, I want to allow data URL navigations if they are initiated by an iframe whose top frame is also a data URL. This is so that frames on data URLs can navigate to other data URLs. Doing that requires checking the top frame URL of the initiator instead of the frame for me (but I think jialul@ is interested in the actual initiator frame itself, so we'll need both?).
We basically don't have this information for OOPIFs. Even if we did have it, I'm a bit hesistant to plumb out the initiator's top frame's URL as well--that's a lot of additional state on NavgationHandle. =(
For safe browsing purpose, I'm OK with only knowing the top frame URL of the initiator too, if that makes things easier. 
Per comment 25, it makes things harder =(
Blocking: -632514

Comment 29 by vakh@chromium.org, Jun 2 2017

Labels: -Pri-1 Pri-2
Labels: Hotlist-EnamelAndFriendsFixIt
Status: WontFix (was: Started)
Per comment #25, and since this issue no longer block 632514. Mark it as won'tfix.

Sign in to add a comment