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

Should request's origin be exposed to javascript #272

Closed
mkruisselbrink opened this issue Apr 4, 2016 · 16 comments
Closed

Should request's origin be exposed to javascript #272

mkruisselbrink opened this issue Apr 4, 2016 · 16 comments

Comments

@mkruisselbrink
Copy link
Collaborator

A request has an associated origin (usually the origin of the client that made the request). Currently for foreign fetch we expect the foreign fetch event handler to pass this origin to respondWith to explicitly expose the contents of the response to that origin. But the origin of a request isn't actually exposed to javascript. I could add an origin or clientOrigin attribute to ForeignFetchEvent, but maybe it makes more sense to expose this directly for all Request instances?

@wanderview
Copy link
Member

I think origin belongs on the event for a couple reasons:

  1. We already expose the origin on ExtendedMessageEvent, so there is precedence.
  2. A Request can theoretically be constructed in one origin and passed to fetch() in another origin. Would the Request.origin be the origin of construction or something added by the browser when fetch() is invoked? What if a ForeignFetchEvent.request is serialized to Cache, then later pulled back out and used in a new fetch()? What would its origin be then? We could figure all this out, but I think it would be easier to just put the origin on the event.

Overall the FetchEvent and ForeignFetchEvent objects are our way of exposing to script that a network request. Assuming that we care about the origin of the code initiating the request, then I think it makes the most sense to keep the origin on those events.

@mkruisselbrink
Copy link
Collaborator Author

  1. A Request can theoretically be constructed in one origin and passed to fetch() in another origin. Would the Request.origin be the origin of construction or something added by the browser when fetch() is invoked? What if a ForeignFetchEvent.request is serialized to Cache, then later pulled back out and used in a new fetch()? What would its origin be then? We could figure all this out, but I think it would be easier to just put the origin on the event.

A request already has an origin. It is most of the time initialized to "client", and updated by fetch() to match the origin of the client that called fetch(). Very similar to how the referrer of a request is dealt with (similarly initialized to "client', updated to the actual referrer when fetch() is called). And fetch already uses this origin for various CORS related checks, so all implementations should already be keeping track of this origin anyway. My question here is how the origin of the request should be exposed to javascript.

@wanderview
Copy link
Member

Sure, its a book keeping item for fetch() because it doesn't have anywhere else to put it. I'm arguing it would be confusing as an exposed attribute on Request from an API point of view because you can get Request objects from places other than a fetch(). For example, constructing them directly, out of cache, etc.

@mkruisselbrink
Copy link
Collaborator Author

Ah, okay. Yeah, it possibly indeed makes most sense to expose the origin only on the event and not on the request itself, but it doesn't seem that different from for example the referrer. That one has the same complications with getting Request objects from places other than a fetch().

@wanderview
Copy link
Member

Actually, moving the referrer to the event objects would help with issues like #245. Maybe there is some use case for wanting the referrer in the object serialized to Cache, though. I'm not sure how people use referrer.

Also, I guess you can override the referrer when creating a request and performing a fetch(). That is different from origin. I don't think we would ever want someone to change the origin of a request manually through a script exposed API.

@annevk
Copy link
Member

annevk commented Apr 5, 2016

It would be a little weird though to expose referrer in one place and origin in another.

@wanderview
Copy link
Member

It would be a little weird though to expose referrer in one place and origin in another.

Referrer can be directly initialized in the Request constructor? AFAIK, origin cannot be initialized like that. Its just always going to be "client" after construction, right?

Not sure if its a real problem, but exposing origin would allow script to tell if a Request was ever fetched or not. Exposing this kind of transient state on the Request seems to muddy its API from a "object that represents a potential network request" to something like "object that represents a potential or maybe already invoked network request".

Incidentally, if a service worker does fetch(foreignFetchEvent.request), does the fetch() use the foreign origin or is it coerced to the service worker's origin somehow?

@mkruisselbrink
Copy link
Collaborator Author

Incidentally, if a service worker does fetch(foreignFetchEvent.request), does the fetch() use the foreign origin or is it coerced to the service worker's origin somehow?

That's a very good question... As currently spec'ed I believe it is coerced to the service worker's origin. The fetch() method invokes the Request constructor with the arguments passed to it, which causes a new request to be created that is mostly identical to the request passed in, but with origin set to "client" (and possibly a few other differences as well). As spec'ed it would keep the original referrer though (so referrer and origin won't match). I think this all works out the way I would expect it (the fetch from the service worker being same origin, I think the fetch algorithms would do the right thing).

@annevk
Copy link
Member

annevk commented Apr 6, 2016

@wanderview isn't that true for referrer as wel? request.referrer can return about:client. Presumably origin would return the empty string or some such in such cases.

As for your example, not sure. In a way it might make sense to use the foreign origin there so the server knows that this is not a normal same-origin request.

@mkruisselbrink
Copy link
Collaborator Author

Thinking about this more I think exposing it on the ForeignFetchEvent rather than just the Request would be really weird, as you could just call new ForeignFetchEvent({request: r}).origin anyway to get the origin out of any arbitrary request. So having the getter on the event wouldn't actually change the requests for which the origin would be exposed, it just slightly obscures it for some.

@wanderview
Copy link
Member

Thinking about this more I think exposing it on the ForeignFetchEvent rather than just the Request would be really weird, as you could just call new ForeignFetchEvent({request: r}).origin anyway to get the origin out of any arbitrary request. So having the getter on the event wouldn't actually change the requests for which the origin would be exposed, it just slightly obscures it for some.

I think this depends on when the origin is set, no? The ForeignFetchEvent.origin could be set during dispatch.

I continue to dislike putting mutable state on Request, but I'll concede if you all want it there.

@mkruisselbrink
Copy link
Collaborator Author

I think this depends on when the origin is set, no? The ForeignFetchEvent.origin could be set during dispatch.

I suppose that's true, although it seems a bit weird to sometimes not have the origin match the requests origin. But then it's not like a script-created foreign fetch event is going to do anything usefull anyway, so the weirdness is probably not so bad.

Okay, for now I'll add an origin attribute to ForeignFetchEvent, which is initialized on dispatch rather than from from the request.

mkruisselbrink added a commit to w3c/ServiceWorker that referenced this issue May 12, 2016
As discussed in whatwg/fetch#272, this exposes the request's origin as
an attribute separate from the request.
@annevk
Copy link
Member

annevk commented May 16, 2016

@wanderview you didn't really address my question though. Request already has fields that "mutate" as a request traverses from one global to another.

@wanderview
Copy link
Member

@wanderview you didn't really address my question though. Request already has fields that "mutate" as a request traverses from one global to another.

Yes, I don't like that. I'd prefer not to go further down that road.

I also said I would concede if you guys are in agreement with each other and want to expose it on Request.

@mkruisselbrink
Copy link
Collaborator Author

Perhaps related to this is w3c/ServiceWorker#899. For no-referrer requests we almost certainly don't want a foreign fetch handler to be able to read the origin of the request. So depending on if foreign fetch should be able to intercept no-referrer requests at all, the origin that is exposed to javascript might not always be able to be the same as the requests origin.

@annevk
Copy link
Member

annevk commented Nov 24, 2017

I'm going to close this given it seems very much foreign fetch related. If there's another reason we should consider this please file a new issue, thanks!

@annevk annevk closed this as completed Nov 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants