Skip to content

Make "no-referrer" censor cross-document AppHistoryEntry URLs #189

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

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

domenic
Copy link
Collaborator

@domenic domenic commented Nov 17, 2021

Also be clear that, despite the shaky spec foundations, the intent after this fix is to expose session history entries across browsing context group swaps.

Closes #71.


Preview | Diff

Also be clear that, despite the shaky spec foundations, the intent after this fix is to expose session history entries across browsing context group swaps.

Closes #71.
Copy link

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so the idea is that if you're on the document with no-referrer you get to see everything, but elsewhere you do not get to see the URL. But elsewhere you do get to see key/id/index/state? Is that what we want or should we similarly protect those?

1. If [=this=]'s [=relevant global object=]'s [=associated Document=] is not [=Document/fully active=], then return the empty string.
1. If [=this=]'s [=relevant global object=]'s [=associated Document=] is not [=Document/fully active=], then return null.
1. Let |she| be [=this=]'s [=AppHistoryEntry/session history entry=].
1. If |she|'s [=session history entry/document=] does not equal [=this=]'s [=relevant global object=]'s [=associated Document=], and |she|'s [=session history entry/policy container=]'s [=policy container/referrer policy=] is <a>"`no-referrer`"</a>, then return null.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: no need for comma before and (while it looks like a lot of words, that's mostly in the source).

@domenic
Copy link
Collaborator Author

domenic commented Nov 18, 2021

I don't think we want to protect those:

  • key/id are browser-generated UUIDs and not sensitive information. Prohibiting access to key would also prohibit traversal back to that entry which seems suboptimal since it'd force people to using history.go().
  • index is trivially computable since x.index is just a convenience getter for to appHistory.entries().indexOf(x)
  • state is something you affirmatively opt in to storing; see previous discussion in Scope app history entries to the browsing context group #71 (comment)

@annevk
Copy link

annevk commented Nov 18, 2021

I was thinking you might want to have private state just for history entries on that document. I think I'm okay with not going to those lengths though.

@domenic domenic merged commit 7c0332b into main Nov 18, 2021
@domenic domenic deleted the browsing-context-group branch November 18, 2021 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scope app history entries to the browsing context group
3 participants