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

Redesign request's context and context frame type features #93

Closed
annevk opened this issue Jul 23, 2015 · 48 comments
Closed

Redesign request's context and context frame type features #93

annevk opened this issue Jul 23, 2015 · 48 comments

Comments

@annevk
Copy link
Member

annevk commented Jul 23, 2015

One that returns one of "navigation", "worker", or "resource".

See w3c/ServiceWorker#673 for earlier discussion.

Tentative proposal is contextSet since I'd like to avoid both "group" and "type" in the name. Any better ideas?

@wanderview
Copy link
Member

Do we really need a new enum for this? I think I would prefer something like:

if (req.navigation)

It would also sidestep the hassle of naming a collection of contexts, etc.

@annevk
Copy link
Member Author

annevk commented Jul 23, 2015

I think you're correct. isNavigation is fine. That way we could have isClient down the line too, or something like isImage. Without "is" seems problematic, since we might get request.client and who knows what other names we need there for variables.

@annevk
Copy link
Member Author

annevk commented Jul 27, 2015

An alternative that @sicking suggested is to group all navigation-like contexts into a single one. Since there's not much point distinguishing a hyperlink from setting location.href.

I believe the main case is <form> for CSP, but I would need to check.

Also, if we go down that route https://code.google.com/p/chromium/issues/detail?id=455116 needs to be notified.

@annevk
Copy link
Member Author

annevk commented Jul 27, 2015

So if we'd have "navigate", "internal" from https://fetch.spec.whatwg.org/#navigation-request-context would be restricted to non-navigation fetches. We'd need a "context element" to handle form/object/embed for CSP, but that does not need to be exposed.

I think @mikewest might slightly prefer this model too. Anyone else opinions?

@ehsan
Copy link

ehsan commented Jul 27, 2015

I have been implementing the various RequestContext types in Gecko, and based on my experience, I think we need to pause and agree on what RequestContext is supposed to represent first.

Currently, RequestContext represents values on three axes:

  • What a specific thing that is being loaded from the network is (for example: "font")
  • How a specific thing is being loaded (for example: "hyperlink")
  • Where (as in, what specific tag, for example) a request is coming from (for example: "frame" vs. "iframe")

This can lead to a number of difficult questions to answer. For example: "How should we represent an audio file being downloaded from a

As far as I can tell, there are two things that make these questions difficult to answer:

  • It is not clear which one of these axes a specific RequestContext value appears on (for example, the question about loading audio inside a
  • It is not clear what specific value should be used in specific cases, since the respective specs usually are not based on top of Fetch.

I think it may make more sense to split the first two axes into two completely separate enums. The third axis seems not too valuable to support, as far as I can tell.

FWIW, in Gecko, I'm planning to disable RequestContext completely so that we don't have this as a blocker to ship service workers, until the spec situation gets resolved.

@sicking
Copy link

sicking commented Jul 27, 2015

Thanks @ehsan. That matches my thoughts almost exactly. I'm not sure if there's value in the second or third bullet. But adding properties for those seems like it can be done as use-cases come up.

The other thing to keep in mind that when "what is being loaded" is "a document" then we don't actually know that the resource will be a HTML document. A URL being loaded into a top-level window or into an <iframe> can be at least a HTML/XML/SVG document, an image, a audio file, a video file, a plugin, or a pdf. Or probably a bunch of other things. This is fine, but just something we should keep in mind to either clearly document, or to make it clear when naming the enum-value for navigation.

@igrigorik
Copy link
Member

@ehsan @sicking not claiming this is complete or accurate, but after staring at the Fetch "context" table for a bit, came up with the following...

Request {
    enum ResourceType { // CSP        "Context"
        document,
        stylesheet,     // style-src  (style)
        script,         // script-src (script)
        image,          // img-src    (image, imageset)
        media,          // media-src  (audio, track, video)
        font,           // font-src   (font)
        object,         // object-src (applet, embed, object)
        other
    },

    enum Initiator {
        parser,
        css,            // style-src?
        script,         // connect-src (fetch, xhr, beacon, eventsource)
        internal        // connect-src (cspreport, nelreport, ping)
    }

    enum Nested{
        none,
        frame,          // child-src
        iframe,         // child-src
        worker          // child-src
    }
}
  • ResourceType would cleanly solve the use-case in Mechanism to indicate "destination context" #64 -- just ignore the "context" part of that discussion.
  • Separating out initiator does allow to ask new and interesting questions. For example, what's the policy for CSS-initiated image fetches vs parser or script initiated ("same" is valid answer).
  • "Nested" is a bit odd but we do have a CSP policy around it and it also is an interesting bit of information to have for prioritization.

@annevk
Copy link
Member Author

annevk commented Jul 29, 2015

I think something like that setup can work, but I think it needs to be more granular. We want different Accept headers for "video" vs "track" for instance.

Also, "worker" is a type, just like document, and I think we want to distinguish the various types of workers, though we could do that on a distinct axis as well.

If @mattto could indicate whether Chrome can change I can create a new proposal.

@mikewest
Copy link
Member

  1. Let's assume Chrome can change.
  2. I agree with @annevk that we need more granularity in the ResourceType enum. For instance, image is "optionally-blockable" for the purposes of mixed content, while "imageset" is blockable. Likewise, track is blocked, while audio and video are optionally-blocked. Splitting these out allows us to be more aggressive about blocking things than we could be otherwise.
  3. It's not clear to me what distinctions Initiator is meant to express. It's also not clear that it gives us enough data to replicate connect-src, but whatever. We can figure that out.

@sicking
Copy link

sicking commented Jul 29, 2015

I agree that we'll need need different types for track, audio and video. I'm not convinced that we should have different types for script and worker. Both of those process the returned resource in largely the same way. The main difference is on which thread they execute.

I think the initiator isn't entirely right. I was thinking of something more like (for document loads) meta-refresh, hyperlink, location-api, form, (for image loads) css-background, css-image, img-element, (for scripts) script-element, shared-worker, importscripts-api, etc. This is a very incomplete list obviously.

I'm honestly not sure that it's worth exposing at this time. Seems like a lot of work to define and I'm not convinced of the value. It seems unlikely to me to help implementations implement CSP.

@mikewest
Copy link
Member

script and worker also differ in terms of their capability, as the former has access to the DOM, and the latter has it's own execution context and therefore its own Content Security Policy. I'd suggest that distinguishing between them is reasonable.

For CSP and MIX, I don't think we particularly care how the request was initiated. We care more about how the content will be used, and we're using things like "The request came from an <img> tag" as a proxy for that detail.

@annevk
Copy link
Member Author

annevk commented Jul 30, 2015

Yeah, worker needs to be distinct.

CSP needs initiator for form-action. I think that's the only value we'll need for now and it does not need to be exposed to script.

@annevk
Copy link
Member Author

annevk commented Jul 30, 2015

So I'm convinced that document is better than a bunch of values that are actually initiators of sort. And that "type" is a better name than "context". However, I still think we want to be specific where we can with types, for reasons @mikewest pointed out.

For XMLHttpRequest, fetch(), rel="prefetch", etc. we should probably have an any type.

I think the different workers each deserve their own type since they represent different global objects, but I might be able to be persuaded. I think combined with my previous comment this gives me enough to go on to specify a new model tomorrow.

@sicking
Copy link

sicking commented Jul 30, 2015

I think we're starting to mix the three orthogonal types again. I agree that it's useful to be able to tell that something is a lot for a worker. But I think worker is a "where is it used", not a "what type is the resource".

Likewise the different worker types are "where is it used" and not different types of resources.

@annevk
Copy link
Member Author

annevk commented Jul 30, 2015

I think worker, sharedworker, serviceworker, and document have equal footing in that respect.

@igrigorik
Copy link
Member

Another attempt...

Request {
    // What type is the resource?
    enum ResourceType {
        document,
        font,          
        stylesheet,    
        script,        
        image,         
        video,         
        audio,
        track,
        object,        
        any
    },

    // How the content will be used?
    // ... consumer? receiver? 
    enum Initiator {
        link-prefetch,
        link-preload,
        link-prerender,
        link-import,
        script,
        style,
        img,
        picture,
        ...,

        beacon,
        form,
        hyperlink,
        location,
        ...
    }

    // What context initiated the request? Maybe?
    enum ??? {
        plugin,
        parser,
        style,
        script,
        worker,
        sharedworker,
        serviceworker
    }
}

@annevk
Copy link
Member Author

annevk commented Jul 31, 2015

@igrigorik the img/picture distinction does not help, since img can have srcset without picture. Also, I don't understand why document makes sense, but worker doesn't. They're identical.

@mfalken
Copy link

mfalken commented Jul 31, 2015

Chome's certainly willing to change once the discussion settles. I think we can remove/deprecate in the meantime too: https://code.google.com/p/chromium/issues/detail?id=515786

I don't feel I have a good enough sense to be useful in this spec discussion though. cc: @jakearchibald @slightlyoff

@sicking
Copy link

sicking commented Jul 31, 2015

Don't we at this point all agree that we shouldn't expose the Initiator to script? So discussing which Initiator types exist seems pretty pointless, no?

A worker essentially has exactly the same processing model as a script. The only difference is that right before evaluating the script a new global is created. But after that global has been created, the worker is processed exactly the same way that a script is. So the security implications of processing a worker is the same as a script. The caching policies that you'd want to have for a worker matches what you'd want to use for a script. The templating or transpiling that you'd want to do for a worker matches what you'd want to do for a script. The URL policies that you'd want to use for a worker is the same as you'd want to use for a scripts and other subresources.

The only other difference that I could see between a worker and a script is that we allow cross-origin loads of scripts but not of workers. Though to be honest that's sort of silly that we have that limitation and something that would be good to fix (possibly with exception of ServiceWorkers).

Documents on the other hand handle entirely different content types. Any time that a document is loaded a navigation is done, which means that redirects have a very different effect. The cache policy you'd want to use is likely different. You're much more likely to want to use human-readable URLs for a document than for a worker (again, possibly ServiceWorkers excepted). Etc.

@annevk
Copy link
Member Author

annevk commented Jul 31, 2015

Actually, worker requests go through their own service worker, just like documents. And currently workers can only be JavaScript, but soonish they'll be able to be WebAssembly too, I'm sure. Just like documents can be HTML, images, plugins, etc.

@sicking
Copy link

sicking commented Jul 31, 2015

When workers can go through WebAssembly, then scripts will be able to as well, no?

@annevk
Copy link
Member Author

annevk commented Jul 31, 2015

No, only module loads most likely. But we'll have a type of worker that's also a module. (Has to be opt-in unfortunately since the syntax is incompatible...)

I think the most distinguishing bit of worker is that it ends up with its own environment, which is also the most distinguishing bit of document. And both get to select their service worker based on their URL, rather than the URL of something they're associated with.

@annevk
Copy link
Member Author

annevk commented Jul 31, 2015

@sicking also, the reason we're discussing initiator is that I'm not sure the image/imageset stuff should be initiator. Or whether icons should be image or something else. (Agreed though that we don't need to expose initiator for now.)

@sicking
Copy link

sicking commented Jul 31, 2015

Maybe a better place to start this discussion is: What is the usecase for exposing a request type at all?

@annevk
Copy link
Member Author

annevk commented Jul 31, 2015

For exposing it to script these cases have come up:

  • Revealing to the service worker whether something is a navigation. (I think there might be value for other types too, if you have e.g. transpiling going on for images and you don't want to have to inspect the response before setting up that code.)
  • As a convenient way to influence Accept and the user agent's notion of "priority" for fetches initiated through fetch() and various preloading mechanisms.

And to a lesser extent the idea has been to expose to script the same hooks that CSP, Mixed Content, etc. get so you can implement your own policies in service workers.

@jakearchibald
Copy link
Collaborator

Yeah, @annevk's use-cases are right. More specifically:

  • User navigates to an image. No matching cache entry, network fails. I want to serve my "Ooops, not available offline" page
  • Page loads same image as an image. No matching cache entry, network fails. I want to serve a placeholder image.

Also if preload can set context, we should be able to do it via new Request too, although it's not clear to me if <link rel="preload" href="/blah" as="image"> produces a request with context "image", or a request with context "preload" that otherwise looks like an image request (Accept etc).

Edit: Ok, the above is addressed by separating out type & initiator. Got it.

@annevk
Copy link
Member Author

annevk commented Jul 31, 2015

Note also that for the purposes of CSP, Mixed Content, etc. these values need to be immutable. Which means rel=preload et al cannot set them. (Alternatively some kind of protection needs to be added on the response so that it's only used for the stated purpose. Or we have two sets of identical values assigned differently.)

@jakearchibald
Copy link
Collaborator

I'm going round and round in circles on this. Here are some thoughts at least:

In order to correctly serve a fallback, we're looking for a hint as to how the response will be processed. A page navigation is "any" in this case. This is distinct from content-types as an <img> will accept a resource of text/html but process as an image. For this particular use-case, isNavigation gets us most of the way there.

Preload wants a hint for the request type, so it can set correct headers and match the request up later.

CSP wants a hint as to how the response will be processed, but also wants the initiator in the case of forms.

Based on CSP, here are a set of processors:

  • connect
  • font
  • img
  • media
  • object
  • script
  • style
  • other

Specs using fetch should use "other" if a new type isn't necessary for CSP (eg, "manifest").

Additionally, we need to know the type of client this response creates:

  • document
  • child-document
  • worker
  • sharedworker
  • serviceworker
  • subresource (does not create a client)
  • unknown (for objects without typemustmatch)

…or no value, for a non-client request.

To cover the edge cases, CSP gets a bit to indicate a form submission, although it'd be great if it were redefined in terms of post/get requests the page could make. child-src can be determined through the client-creation type, script-src can be determined by "script" that doesn't create a client.

Preload as would take the processing types and map them to request types in terms of as.

ServiceWorker can determine navigation and framing through "document" and "child-document", and can serve a fallback image for "img". Not sure context-frame is needed.

@sicking
Copy link

sicking commented Jul 31, 2015

  • Revealing to the service worker whether something is a navigation. (I think there might be value for other types too, if you have e.g. transpiling going on for images and you don't want to have to inspect the response before setting up that code.)
  • User navigates to an image. No matching cache entry, network fails. I want to serve my "Ooops, not available offline" page
  • Page loads same image as an image. No matching cache entry, network fails. I want to serve a placeholder image.

For this both the .isNavigation = true and the .context = "document" proposals would work, right? And how we would treat workers wouldn't affect this use case?

  • As a convenient way to influence Accept

This would seem like an argument for treating workers as script, and for treating all image types as a single type, no matter if they are icons, image-set, <img> or <picture>.

  • the user agent's notion of "priority" for fetches initiated through fetch() and various preloading mechanisms.

Does any of the proposals made so far help with this one? Other than possibly having a very fine-grained "initiator" exposed to script, but that doesn't seem like something that we want to do?

Regarding CSP, I don't see how exposing anything to webpage script is helping here. Am I missing something? Is the proposal that the service worker would somehow take over the responsibility of enforcing CSP?

@jakearchibald
Copy link
Collaborator

For this both the .isNavigation = true and the .context = "document" proposals would work, right? And how we would treat workers wouldn't affect this use case?

If I knew the request was going to create a document or child-document client, I wouldn't need .isNavigation.

(influencing Accept) would seem like an argument for treating workers as script, and for treating all image types as a single type

Agreed. We may want to expose a more detailed initiator in future, but it seems simpler as a starting point to go with what CSP and ServiceWorker already define.

Does any of the proposals made so far help with (priority)?

It depends how fine-grained prioritisation needs to be. Do browsers treat <img> and background images differently priority-wise? Prioritisation is unspecced, but the browser can attach whatever private data to a request object, once that's used in a call to fetch(req) the browser can use that priority.

Is the proposal that the service worker would somehow take over the responsibility of enforcing CSP?

Nah, I'm only talking about CSP to show that the idea is building upon what's already specced elsewhere, whereas a granular "initiator" would be a new and big thing.

@annevk
Copy link
Member Author

annevk commented Aug 3, 2015

As for "child-document" vs "document", one concern is that sometimes you want to have different treatment for being a "child-document", e.g. a same-origin "child-document" would not want to show offline UI. But a cross-origin "child-document" would want to show such UI. It might be better therefore to just expose it as "document" but also expose "ancestorOrigins" somehow on request, which we've added to Window for much the same reasons.

@jakearchibald
Copy link
Collaborator

No arguments here

@jakearchibald
Copy link
Collaborator

Related: NEL reporting is likely to be a client request that doesn't result in the creation of an environment settings object w3c/network-error-logging#54 (comment)

@annevk
Copy link
Member Author

annevk commented Aug 10, 2015

So it seems we need quite a few things to replace context and context frame type:

  • request's type (image, scripts, video, etc.)
  • request's result type (document, worker, sharedworker, resource etc.)
  • request's browsing context ancestor origins (for offline UI)
  • request's initiator (form, picture, img; for CSP and Mixed Content)

Of those we would expose the first three to service workers. Does this make sense? Should I try to work this out?

@sicking
Copy link

sicking commented Aug 11, 2015

Sounds great to me. I might call the second one something like
"destination" rather than "request result type", but I won't bikeshed.

The only thing that might be controversial is origins of ancestors. I know
that has been debated to be exposed elsewhere but there has been privacy
concerns.

I'm personally fine with exposing it though. Possibly we could have a way
for a website to indicate that it wants to be anonymous to its descendants
in which case we'd stick a " null" in the anceastor chain and cut it off
there.

But I don't have a strong opinion on if that should be needed. Nor on if
that should be opt-in or opt-out.

@annevk
Copy link
Member Author

annevk commented Aug 11, 2015

I wrote up https://html5.org/temp/fetch-newcontext.html as a proposal. Attempting to find something that works for CSP and Mixed Content as well, though @mikewest is not very happy about the shorter lists so far.

http://krijnhoetmer.nl/irc-logs/whatwg/20150811#l-252 has some questions and feedback from @jakearchibald.

@mikewest
Copy link
Member

I think I'd say that I can live with the current list, but it seems like it's going to require some mental gymnastics when writing/reading the CSP spec to understand why a particular combination of properties means script-src as opposed to whatever-src. Having a massive list of the ways in which requests could be generated (e.g. a much more granular "initiator") was a simpler mental model.

@annevk
Copy link
Member Author

annevk commented Aug 11, 2015

I would be open to adding more initiator types to help out CSP and (potentially) Mixed Content. My main worry is interaction cases, such as a <form> loading something in <object>, which the current model breaks on. This was simply the minimum set I could find that seems to work for all current specifications.

@annevk annevk changed the title Introduce convenience property for request's context Redesign request's context and context frame type features Aug 11, 2015
@igrigorik
Copy link
Member

@annevk looking at that table, does it mean that fetch (initiator) is only allowed to have empty type? What about the use case of "fetch this as an 'image'"?

Also, what exactly is subresource? Should that be replaced by preload & prefetch (these two need to be separate).

@annevk
Copy link
Member Author

annevk commented Aug 11, 2015

@igrigorik "as image" would require a new "as type" axis (equivalent to "type"). We cannot allow any of these to be set because they are also used for security checks. We went through this a few times...

A "subresource" destination is one whose service worker is selected based on the request's client. E.g., <img>, <link rel=stylesheet>, have that.

@igrigorik
Copy link
Member

We cannot allow any of these to be set because they are also used for security checks. We went through this a few times...

Yes, and as we discussed before, can we unbundle that? When I say "fetch this as X" I'm specifically thinking of negotiation + prioritization use cases -- today's UA's send different HTTP headers and assign different priorities based on ~type; I want to expose this. For security...

  • When I use fetch() the fetch is subject to connect-src, regardless of type value.
  • When I use <link rel= preload / prefetch / prerender> those are subject to own x-src (TBD), regardless of type value.

Perhaps we're missing an extra column in there to address the security angle.. and the CSP directive prefixes are basically exactly that: object, media, font, script, etc.

@annevk
Copy link
Member Author

annevk commented Aug 12, 2015

No, CSP directives are a weird mix of things. Not at all principled. As I said, the "unbundling" is basically copying the "type" column as an "as type" column.

@mikewest
Copy link
Member

CSP is totally principled! The principle is "These are things we want to allow developers to do. So let's stuff them in." :)

Where does pre{load,fetch,connect,render} fall in this categorization, BTW?

@annevk
Copy link
Member Author

annevk commented Aug 12, 2015

Do we have their processing models defined in sufficient detail? Last I checked it wasn't quite clear how they would actually work in terms of Fetch and HTML's navigate algorithm.

@mikewest
Copy link
Member

I have no idea, but they're another of those things I want to allow developers to control, so I want to stuff them into CSP3 somehow sanely.

@annevk
Copy link
Member Author

annevk commented Aug 12, 2015

@mikewest if there's no processing model though it's unclear what kind of control would be good.

@mikewest
Copy link
Member

I agree with you. :) I'm just pointing out the gap.

@annevk annevk closed this as completed in d2208fa Aug 12, 2015
@annevk
Copy link
Member Author

annevk commented Aug 12, 2015

Once we figure out the processing model for pre* please file a new issue and we'll get them integrated.

rakuco pushed a commit to crosswalk-project/blink-crosswalk that referenced this issue Aug 28, 2015
> [Fetch] Add a deprecation warning for Request.context
> 
> Request.context is removed from the spec:
> whatwg/fetch#93
> and we will remove it in M-46.
> 
> This CL adds a deprecation warning to M-46 a little before Request.context is
> actually removed by https://codereview.chromium.org/1292503002/, and
> is to be merged into M-45.
> 
> BUG=515786
> 
> Review URL: https://codereview.chromium.org/1287273002

TBR=hiroshige@chromium.org

Review URL: https://codereview.chromium.org/1296143002

git-svn-id: svn://svn.chromium.org/blink/branches/chromium/2454@200626 bbb929c8-8fbe-4397-9dbb-9b2b20218538
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

8 participants