Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

What kind of new 'session' is created on manual navigation? #6356

Open
jakearchibald opened this issue Feb 2, 2021 · 24 comments
Open

What kind of new 'session' is created on manual navigation? #6356

jakearchibald opened this issue Feb 2, 2021 · 24 comments
Labels
interop Implementations are not interoperable with each other topic: history topic: navigation

Comments

@jakearchibald
Copy link
Collaborator

jakearchibald commented Feb 2, 2021 •
edited

Some APIs and behaviours span across multiple session entries. But, if the page is navigated to a new entry by a means out of the current page's control (user enters something into the URL bar, selects a bookmark etc etc), which of these behaviours should continue across into the new page?

I'm going to refer to that type of navigation as "manual".

WindowProxy window references

  1. PageA uses window.open to open PageB in a new session.
  2. PageB's session is manually navigated to PageC.

Does PageA hold a reference to PageC?

Chrome: No if PageC is on a different origin to PageB, otherwise yes (unless PageC is cross origin isolated via COOP/COEP).
Firefox: Yes, unless PageC is cross origin isolated via COOP/COEP
Safari: Yes

Ideal behaviour: This was discussed in #5767 (comment), and consensus was that PageA should not hold a reference to PageC in the cross-origin case, and this should be achieved by creating a new BCG for PageC. Although, it isn't yet clear what should happen to the reference PageA has, or what should happen if PageC's session is navigated back to PageB. We didn't discuss the same-origin case.

history.back and history.length

  1. PageA's session is manually navigated to PageB.

Can PageB navigate its session back to PageA using history.back? Is history.length greater than 1?

All browsers: Yes.

Ideal behaviour: ???

It feels like a minor security and privacy benefit to treat PageB as part of a new sub-session, that cannot observe or control history entries that came before it. Whether this differs if PageA and PageB are same-origin probably depends on what we do for the previous example.

Chrome already blocks history.back() in some cases, eg when it's called from a sandboxed iframe and the navigation would happen outside the iframe. I guess even a navigation within the sandboxed iframe impacts history.state in the parent, but we already know the history API is bad.

sessionStorage

  1. PageA has some session state.
  2. PageA's session is manually navigated to PageB.
  3. (if necessary) Use location.href to navigate back to PageA's origin.

Can the resulting page access the session state set in PageA?

All browsers: Yes.

Ideal behaviour: ???

It feels like the answer here should be the same as it is for history.back.

Related: Session storage and changing browsing contexts.

cc @annevk @domenic since we've already been talking about this stuff
cc @mikewest anyone from security interested in weighing in here?
@mikewest
Copy link
Member

mikewest commented Feb 2, 2021

Poking folks who know things, and who might be interested in weighing in.

@asakusuma
Copy link

Another similar scenario is "right click link, open new tab." For same-origin scenarios, it would be helpful for the origin and destination page to have references to each others WindowProxy objects. Consider a search page, with a bunch of results. Users will often scroll through and right click, new tab all the results that look interesting. From a UX perspective, all of the tabs are part of the same activity, a user searching, and it's helpful to be able to share state. Right now, I believe the best to handle this is to use query params with some identifier as lookup key in persistent storage. But nobody likes query params in their urls.

@jakearchibald
Copy link
Collaborator Author

@asakusuma I guess a service worker could create that association. Also, we've thrown around the idea of having some sort of clients API outside of service workers.

@tabatkins
Copy link
Collaborator

tabatkins commented Feb 2, 2021 •
edited

Purely from a personal perspective, I feel strongly that manual navigation should create an entirely new session. Typing into the URL bar should be semantically identical to closing the tab, opening a new tab, and then typing into the URL bar. Anything less than that is at least a privacy leak, and possibly a security leak, imo.

(At least when the new page is cross-origin. I don't have strong opinions on same-origin.)

@ArthurSonzogni
Copy link
Member

Yes, I believe every "manual" navigation (Bookmarks, address bar, ...) should start a new Browsing Context Group. WindowProxy window reference shouldn't cross two Browsing Context Group, they should be closed. It would be lovely if the history seen from the new document and the SessionStorage could start from scratch.

WindowProxy window references

I filled several security issues against Chrome and Firefox in the past. They contains many useful informations that you can check out:

In Chrome (and I believe in Firefox too), we severe the WindowProxy relationship whenever we enter a new Browsing Context Group. I do believe we should continue to this way. That's even part of the specification for COOP+COEP.

In Chrome, this happens for:

  • Various security reasons (accessing an internal page, accessing the web store, some error page, ...)
  • When crossing different COOP+COEP contexts.
  • cross-origin navigation with no related previously related Browsing Context:
    ProactivelySwapBrowsingContextGroup
  • same-origin navigation with no previously related Browsing Context, when it helps with the BackForwardCache.
  • ...

More informations in RenderFrameHostManager::ShouldSwapBrowsingInstancesForNavigation(...)

I do believe we would like to always swap Browsing Context Group for "manual" navigation. This is tracked in https://crbug.com/1044951 and I believe +@carlscabgro was interested implemented this.

The main issue: If users navigate back, we won't be able to restore the WindowProxy correctly. Once it's broken, it's broken. That's why we sometimes require having no related Browsing Context before swapping Browsing Context Group. I do believe this is not a huge problem, compared to what we gain from a security perspective. COOP+COEP is a precedent into the specification about not restoring WindowProxy. So I believe this is now easier to go further into this direction.

history.back and history.length

Yes, this would be better to start them from scratch when entering a new Browsing Context Group. The new document shouldn't be able to history.back() across them. However the user should still be able to navigate back across several Browsing Context Group.

@jakearchibald
Copy link
Collaborator Author

@ArthurSonzogni

The main issue: If users navigate back, we won't be able to restore the WindowProxy correctly. Once it's broken, it's broken.

Yeah, I'm pretty sad about that. Some of that was discussed in #5767 (comment). I'd prefer it if 'back' worked here, but it's not a hill I want to die on.

Yes, this would be better to start them (history and session storage) from scratch when entering a new Browsing Context Group.

That was my original thinking, but I think it conflates two things.

  1. User is on PageA
  2. User clicks link which takes them to PageB, which is cross-origin isolated
  3. User clicks link which takes them to PageC, which is on the same origin as PageA and is not cross-origin isolated

In this case all three pages exist in their own browsing context group (yeah?), but since all navigation was controlled by web content, I'd consider it to be part of the same browsing session. PageC would have access to the same sessions storage as PageA, and history.back() would work for both PageB and PageC.

In terms of modeling this, a browsing context group is part of a session, but multiple browsing context groups may exist within the same session. Does that make sense?

@annevk
Copy link
Member

annevk commented Feb 3, 2021 •
edited

One thing I'd like to see clarified in this discussion is whether "manual" is "all manual" or "cross-origin manual". If a user manipulates a path or some such they might not expect to lose session history. (Otherwise I'm mostly aligned with Jake I think, except for restoring WindowProxy. Popups are the worst. 😊)

@jakearchibald
Copy link
Collaborator Author

jakearchibald commented Feb 3, 2021 •
edited

Proposal: When the user manually navigates from PageA to PageB, which have different origins:

  • WindowProxys pointing to PageA's window will not point to PageB (already agreed)
  • PageB will have new session storage.
  • In PageB history.length will be 1, and history.back() will be a no-op.
  • The browser back button will still navigate back to PageA.
  • In PageA history.length will not count PageB's history entry. history.forward() will be a no-op.
  • The browser forward button will still navigate to PageB.

When the user manually navigates from PageA to PageB, which are in the same origin, the behaviour is the same as clicking a link to PageB in terms of the above behaviours. This preserves the 'hackability' of URLs as a form of navigation.

Modelling the above:

A set of contiguous entries in joint session history form a 'session'. There may be multiple sessions within joint session history, but they are always contiguous.

Session storage is associated with a session. If the user navigates into a new session, they get a new session storage.

history.back() and history.forward() are no-ops if they would navigate a navigable into a new session. history.length is the number of history entries in that session. The browser's history controls (back/forward buttons etc) may traverse to history entries with a different session.

Clicking a link in a page creates a history entry in the same session, although it may exist in a different browsing context group due to cross origin isolation etc.

Manual navigations to same-origin URLs behave the same as clicking a link in a page.

@ArthurSonzogni @annevk does that work? Any idea who should weigh in from WebKit?

Manual navigations to cross-origin URLs create a history entry with a different browsing context group and new session.

@jakearchibald jakearchibald mentioned this issue Feb 3, 2021
Merged
8 tasks
@smaug----
Copy link
Collaborator

If PageA has been originally opened using window.open() from Page0, what happens to the reference in Page0?
So basically the same question which is mentioned in the initial comment.

@jakearchibald
Copy link
Collaborator Author

@smaug---- I think that depends on whether navigating back to PageA should restore its connection with Page0. There's some discussion of that in #5767 (comment), but I don't think we need to decide that before getting consensus on the 'manual navigation' stuff.

If we decide that manual navigation creates a new 'session', and therefore new browsing context group, then the window proxy behaviour will be the same as an in-page navigation to a cross-origin isolated page.

@tabatkins
Copy link
Collaborator

When the user manually navigates from PageA to PageB, which are in the same origin, the behaviour is the same as clicking a link to PageB in terms of the above behaviours. This preserves the 'hackability' of URLs as a form of navigation.

Yes, upon further thought I realized I did have strong opinions about same-origin, and wanted it to do exactly this, so that I could navigate myself around a site by hacking the url.

@JakobJingleheimer

This comment has been minimized.

Sign in to view
@jakearchibald

This comment has been minimized.

Sign in to view
@JakobJingleheimer

This comment has been minimized.

Sign in to view
@jakearchibald

This comment has been minimized.

Sign in to view
@domenic domenic added interop Implementations are not interoperable with each other topic: history topic: navigation labels Feb 4, 2021
@rakina
Copy link
Member

rakina commented Feb 16, 2021

This might be a separate question, but should crossing different COOP+COEP contexts cause a session change, or do we only change sessions on manual navigations?

I'm asking this because I think we have different levels of "browsing context group swaps" in Chrome: "proactive" and "mandatory". We will try to do proactive BCG swaps whenever possible (if there are no opener relationships), and this includes same-origin navigations. Meanwhile "mandatory" BCG swaps happen when crossing different COOP+COEP contexts and other security-related stuff.

I think it might make sense to make all "mandatory" BCG swaps to do session swaps also, since there are some stuff that proactive BCG swaps shouldn't do (especially on same-origin navigations):

WindowProxy: seems OK to sever on all BCG swaps, and as Arthur mentioned, Chrome is working on this.
history.back() and history.length: this should still work after proactive BCG swaps, so we should block only when session changes.
sessionStorage: same as history, this should still work after proactive BCG swaps, so we should block only when session changes.

@annevk
Copy link
Member

annevk commented Feb 16, 2021

My preference is to only reset the scope of the History API and session storage (not the back button though) on manual navigations.

@jakearchibald
Copy link
Collaborator Author

Agreed. BCG swaps ensure we don't put no-cors resource bodies in the same process as isolated pages. Sessions as described in this issue are something different. History entries can be part of the same session but their contexts can be in different processes.

@jakearchibald
Copy link
Collaborator Author

@rakina I created a little presentation to explain my thinking here, the relevant part starts 3:42 https://youtu.be/nZb0U3rFQXw?t=222

@rakina
Copy link
Member

rakina commented Feb 17, 2021

OK, so BCG might change and session stays the same. But if session changes BCG will always change. Yeah I think it's reasonable to reset sessionStorage and history API on session changes, and it's more conservative than resetting on BCG swap, which is good. Thank you!

@domenic domenic mentioned this issue Feb 24, 2021
Closed
@domenic
Copy link
Member

domenic commented Mar 8, 2021

FYI #5381 is the issue for clearly speccing this.

@jakearchibald jakearchibald mentioned this issue Apr 12, 2021
Closed
@lauredogit lauredogit mentioned this issue Jul 1, 2021
Open
@jakearchibald jakearchibald mentioned this issue Sep 14, 2021
Closed
@domenic
Copy link
Member

domenic commented Jan 27, 2022 •
edited

It's not 100% clear to me what our proposed preferred behavior is for the same-origin case. This is relevant to #7533 because our current spec and flagged implementation in Chromium for URL bar prerendering causes a BCG swap. (I.e., it creates a new prerendering browsing context in its own separate BCG, and then upon activate, it activates that BC/BCG in place of the old one.) We do this regardless of same-origin or cross-origin.

So concretely:

  • The user is on https://example.com/
  • That page opens a popup to https://example.com/
  • The user goes to the popup's URL bar and types https://example.com/foo, then presses Enter.

Without prerendering for https://example.com/foo: the opener relationship will be preserved. With prerendering: it won't be.

Is that OK?

@annevk
Copy link
Member

annevk commented Feb 15, 2022

That seems rather surprising. I guess ideally we'd always swap, but I'm not sure how feasible that is.

@jakearchibald
Copy link
Collaborator Author

Btw, this definition of a session can span multiple BCGs

@domenic domenic mentioned this issue Mar 17, 2022
Open
@jakearchibald jakearchibald mentioned this issue Dec 15, 2022
Merged
@bnham bnham mentioned this issue Dec 13, 2023
Merged
webkit-commit-queue pushed a commit to bnham/WebKit that referenced this issue Dec 20, 2023
@bnham
Sever window opener on manual navigation
e0471a8 
https://bugs.webkit.org/show_bug.cgi?id=266354
rdar://118951619

Reviewed by Alex Christensen.

When a user does a cross-origin manual navigation (e.g. navigating via the address bar, opening a
bookmark, ...), then we should sever the window.opener reference. This should be safe since other
browsers are already doing this: whatwg/html#6356.

This requires propagating the existing isRequestFromClientOrUserInput flag from more callsites.
Previously we were only setting that flag on NavigationAction after a fragment navigation.

* Source/WebCore/loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::load):
(WebCore::FrameLoader::loadWithDocumentLoader):
(WebCore::FrameLoader::reload):
(WebCore::FrameLoader::loadPostRequest):
(WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
(WebCore::FrameLoader::loadDifferentDocumentItem):
(WebCore::createWindow):
* Source/WebCore/loader/NavigationAction.cpp:
(WebCore::NavigationAction::NavigationAction):
* Source/WebCore/loader/NavigationAction.h:
* Source/WebCore/loader/PolicyChecker.cpp:
(WebCore::FrameLoader::PolicyChecker::checkNavigationPolicy):
* Source/WebCore/page/ContextMenuController.cpp:
(WebCore::openNewWindow):
* Source/WebKit/Shared/NavigationActionData.h:
* Source/WebKit/Shared/NavigationActionData.serialization.in:
* Source/WebKit/UIProcess/API/APINavigation.h:
(API::Navigation::isRequestFromClientOrUserInput const):
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigationInternal):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::createWindow):
* Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp:
(WebKit::WebLocalFrameLoaderClient::didSameDocumentNavigationForFrameViaJSHistoryAPI):
(WebKit::WebLocalFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):
* Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp:
(WebKit::WebRemoteFrameClient::changeLocation):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Canonical link: https://commits.webkit.org/272321@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
interop Implementations are not interoperable with each other topic: history topic: navigation
Milestone
No milestone
Development

No branches or pull requests
10 participants
@mikewest @jakearchibald @asakusuma @domenic @tabatkins @annevk @JakobJingleheimer @ArthurSonzogni @rakina @smaug----