| 07:17 | <annevk> | jugglinmike: https://github.com/web-platform-tests/wpt/issues/18354 might help, though it hasn't been maintained so maybe it's not that useful... |
| 10:05 | <annevk> | littledan: Looking at https://html.spec.whatwg.org/#fetch-a-single-module-script the logical place to enforce the type is before you create a module script. What HTML PR 5658 seems to do is to always respect the server in terms of what to create, which creates the problem that if I import something as JSON and it declares itself as JavaScript, it'll get to fetch further subresources. Or am I misunderstanding something? |
| 10:08 | <annevk> | Ah no, I guess the further fetching doesn't happen |
| 10:08 | <annevk> | But the parsing did, which seems weird |
| 10:23 | <littledan> | annevk: right, I guess since the failure is in "fetch a single module script", it won't recurse until that succeeds. the intention was to be equivalent to just caching the result of the first fetch without interpreting it (so that it wouldn't be retried if the first type supplied was wrong); I agree that we shouldn't let subresources to be fetched in this case. |
| 10:23 | <littledan> | so, no subresource fetching or linking happens until the right type is provided |
| 10:24 | <annevk> | The parse itself seems concerning as well though |
| 10:24 | <littledan> | another way to word this would be to just cache the MIME type and response body, but it turned out that that was observably equivalent to parsing it and storing it in the map eagerly |
| 10:24 | <littledan> | we could rephrase it that other way. i was thinking it might be enough to just write a note explaining that engines could delay the parse if they want to |
| 10:25 | <littledan> | or maybe this is observable because you can see the difference between a parse error and a type check error? |
| 10:26 | <annevk> | It's a little hard to tell for me what the various observable bits are today and also what they might become as add more parsers into the mix |
| 10:27 | <annevk> | as we add* |
| 10:27 | <tobie> | annevk: looking into the PR Preview issue. |
| 10:27 | <tobie> | annevk: seems to be some authentification issue between the app and GitHub. |
| 10:27 | <annevk> | tobie: I tried looking into it myself, but I couldn't really find any recent change that might have done this |
| 10:28 | <tobie> | annevk: yeah, it's something that change for an auth perspective. |
| 10:28 | <annevk> | tobie: that makes sense as it basically doesn't seem to be picking up anything, despite being authorized in the WHATWG org |
| 10:28 | <tobie> | annevk: yes, I'm getting bad credentials all over the place. |
| 10:29 | <littledan> | annevk: That makes sense ,thanks for raising this issue |
| 10:29 | <tobie> | annevk: a bug in the logger hasn't logged those as errors, which is why I didn't hear about them. |
| 10:30 | <annevk> | tobie: a beautiful cascade |
| 10:30 | <tobie> | annevk: right? |
| 10:31 | <tobie> | annevk: https://developer.github.com/changes/2020-04-15-replacing-create-installation-access-token-endpoint/ |
| 10:31 | <tobie> | This might be the issue. |
| 10:32 | <tobie> | annevk: but they're way ahead of their plans if that's the case |
| 10:34 | <annevk> | tobie: yeah, that'd be weird |
| 10:34 | <tobie> | annevk: that's what the error I'm getting points to, though. |
| 10:35 | <annevk> | tobie: I guess update and maybe let GitHub know? Though you'd think they have a working log themselves |
| 10:36 | <tobie> | annevk: yeah, testing first to see if this fixes it. |
| 10:38 | <tobie> | annevk: so I'm clearly onto something here. |
| 10:39 | <tobie> | annevk: can you give it a spin? |
| 10:40 | <tobie> | annevk: please? |
| 10:40 | <tobie> | annevk: seems like it's working for me here. |
| 10:44 | <annevk> | tobie: will do |
| 10:44 | <tobie> | annevk: ty |
| 10:45 | <annevk> | tobie: will it pick up PRs without PR Preview annotations if I edit OP? |
| 10:45 | <annevk> | I made an edit to https://github.com/whatwg/html/pull/5668 just now |
| 10:45 | <tobie> | yes |
| 10:45 | <annevk> | Ah, there it is |
| 10:45 | <annevk> | 😊 |
| 10:45 | <annevk> | ❤️ |
| 10:46 | <annevk> | Thanks tobie! |
| 10:46 | <tobie> | annevk: https://twitter.com/tobie/status/1276103795940958209 |
| 10:46 | <tobie> | annevk: np |
| 10:47 | <tobie> | annevk: filing a couple of bugs to improve the logging for this. |
| 10:47 | <tobie> | annevk: and also automate pr-preview going over failed updates when such a problem occurs. |
| 10:53 | <tobie> | annevk: https://github.com/tobie/pr-preview/issues/74 and https://github.com/tobie/pr-preview/issues/75 |
| 10:54 | <annevk> | littledan: note that if you don't cache the parse, you end up parsing redundantly |
| 12:14 | <littledan> | annevk: Yeah, the idea is that, if the type mismatches, you cache the response body + MIME type, and the first time it's imported with the type matching, then you replace the cache entry with the parsed module |
| 12:17 | <annevk> | littledan: I suppose that might work, even if it's a bit complex; what's the reason to reject Domenic's idea? |
| 12:20 | <littledan> | annevk: The core reason is, we've heard from some JS developers that it'd be confusing to "clone" a module just because it's imported with different attributes; some people have the intuition that there's a 1:1 mapping module specifier <-> module. |
| 12:20 | <littledan> | in the case of `type`, we in fact have no reason to "clone". So it makes sense to use an import condition, which is just a check. |
| 12:22 | <littledan> | so, the intuitions are directly opposite; we just have to choose whether we consider the import conditions to be part of the extended module specifier or not. TC39 got to Stage 2 based on the explicit agreement that they're not part of it, but we could revisit that if needed. |
| 12:34 | <littledan> | you can see the concerns expressed, e.g., in this thread https://github.com/openjs-foundation/standards/issues/91 |
| 12:34 | <littledan> | as well as this one https://github.com/tc39/proposal-import-attributes/issues/30 |
| 12:35 | <littledan> | we're open to a separate proposal which would provide attributes which *do* form part of the extended specifier; it just seems like a semantically separate feature, and not one that we'd use for non-JS module types |
| 12:44 | <littledan> | to me it seems weird to do multiple fetches that are exactly the same, just because the condition applied differs |
| 12:46 | <annevk> | littledan: are they exactly the same or do we use Accept in some way? |
| 12:47 | <littledan> | the current PR doesn't use Accept in any way. Should we use it? |
| 12:47 | <littledan> | this was something I wanted to ask to people who know more about fetch than me |
| 12:47 | <littledan> | in various issue threads, people proposed various HTTP headers that would be affected |
| 12:48 | <annevk> | littledan: it seems like it might be useful for the server, although on the other hand now that it's effectively managed client-side anyway there's not much need for negotiation |
| 12:48 | <littledan> | I can see that that would be a reason to consider it part of the specifier. But I don't see how it's motivated by the initial security issue. |
| 12:48 | <annevk> | it's not |
| 12:48 | <annevk> | I think it's more something we hadn't really considered about modules in general |
| 12:49 | <annevk> | Since it's only fairly recently that we decided to go beyond JS |
| 12:49 | <littledan> | if we passed in an Accept header, it would seem important to consider the type part of the specifier, since the server may override the URL to support multiple different module types, and you wouldn't want caching to block access to that. |
| 12:49 | <littledan> | however, this may be confusing to JS developers, if they expect the same specifier to always point to the same module |
| 12:50 | <annevk> | There's https://wiki.whatwg.org/wiki/Why_not_conneg to not do it |
| 12:51 | <littledan> | oh I wasn't aware of that, thanks for the reference |
| 12:51 | <littledan> | this is also interesting with respect to the ideas about passing more locale information to the server... |
| 12:52 | <annevk> | But there's a lot of push from CDNs around various other formats to do something with conneg... E.g., all of client hints |
| 12:52 | <annevk> | Not necessarily Accept though |
| 14:13 | <annevk> | domfarolino: https://github.com/w3c/webappsec-referrer-policy/pull/137 |
| 14:38 | <domfarolino> | annevk: commented |
| 14:39 | <annevk> | domfarolino: thanks |
| 14:39 | <annevk> | domfarolino: so maybe the problem is that document.referrer isn't currently defined as following Referrer Policy, which seems like a bug |
| 14:40 | <annevk> | or, hmm |
| 14:40 | <domfarolino> | annevk: Yeah, me and Domenic had a discussion about this recently..i think we determined it's not a problem? |
| 14:40 | <Domenic> | I have vague memories of this... |
| 14:42 | <annevk> | domfarolino: for initial about:blank we copy the creator URL: "If browsingContext's creator URL is non-null, then set document's referrer to the serialization of it." |
| 14:43 | <annevk> | For navigation it does seem to account for it |
| 14:44 | <domfarolino> | annevk: I think from the follow-up section in this doc (https://docs.google.com/document/d/18qOHKpUJ1N1GBxfls4XoBfeen09fxrCW0XV4Zs_FWiE/edit?ts=5ea75424#heading=h.hiccz7joqxl0) I determined about:blank iframes should inherit their creator doc's referrer policy too? memory is a bit fuzzy here though |
| 14:45 | <domfarolino> | annevk: Or no, the spec apparently says they should, but maybe they don't? |
| 14:48 | <annevk> | domfarolino: document.referrer is not the policy, but something else; but in browsers it does seem to align with Referer more than it does in the spec |
| 14:49 | <annevk> | e.g., if I open an about:blank popup with no-referrer, then document.referrer reflects that whereas the spec says something else |
| 14:51 | annevk | files https://github.com/whatwg/html/issues/5677 |
| 14:53 | <domfarolino> | annevk: So the issue is that the referrer is the creating document's creator URL, but not censored by its referrer policy? |
| 14:55 | <annevk> | domfarolino: yeah |
| 14:56 | <domfarolino> | annevk: FWIW I added https://github.com/web-platform-tests/wpt/blob/master/referrer-policy/generic/inheritance/iframe-inheritance-about-blank.html |
| 14:56 | <domfarolino> | As a result of filing https://crbug.com.1075729 |
| 14:57 | <domfarolino> | sorry, "." should be "/" |
| 14:57 | <annevk> | that's good to test too, yes; I think the spec handles that reasonably well, except for the part where our policy inheritance is a big mess |
| 14:59 | <domfarolino> | annevk: Right, so if we make any changes to the spec here to censor document.referrer by the creator document's referrer policy, we'll need to change the expectation here https://github.com/web-platform-tests/wpt/blob/master/referrer-policy/generic/inheritance/iframe-inheritance-about-blank.html#L48-L54 |
| 15:01 | <domfarolino> | but yeah, referrer policy inheritance would be cool |
| 15:06 | <annevk> | domfarolino: isn't it censored though? It seems to be in Fx at least |
| 15:07 | <domfarolino> | annevk: I think impls censor it, but the test expects it to not be |
| 15:07 | <domfarolino> | annevk: So if we change the spec to match impls, we'll need to change that test |
| 15:07 | <annevk> | I see, add a comment to the issue I filed? |
| 15:07 | <domfarolino> | annevk: The chrome bug I filed basically says "Chrome censors document.referrer for about:blank iframes, but it shouldn't" |
| 15:07 | <domfarolino> | annevk: Just did |
| 16:01 | <annevk> | domfarolino: I also found out today that Chrome and Safari have a "site"-like referrer policy for anti-tracking purposes |
| 16:01 | <annevk> | domfarolino: whereby https://test.example.com/test?page becomes https://example.com/ |
| 16:02 | <domfarolino> | annevk: How is this RP triggered / used? surely it isn't exposed as a real value for use in `referrerpolicy=...` etc |
| 16:03 | <annevk> | domfarolino: yeah, there's no public API, but maybe there should be |
| 16:03 | <domfarolino> | annevk: When do you see this referrer policy getting used? |
| 16:04 | <annevk> | domfarolino: https://webkit.org/blog/9521/intelligent-tracking-prevention-2-3/ |
| 16:05 | <annevk> | domfarolino: it might be useful for sensitive-343242.registrable-domain.com |
| 16:05 | <devsnek> | if an abort controller is signaled after a request finishes successfully but before the promise resolves |
| 16:05 | <devsnek> | fetch rejects right |
| 16:06 | <domfarolino> | annevk: Do you see chrome mentioned in that or something? |
| 16:06 | <annevk> | domfarolino: I'm not sure I understand |
| 16:07 | <annevk> | devsnek: https://fetch.spec.whatwg.org/#dom-global-fetch |
| 16:07 | <domfarolino> | annevk: You said "Chrome and Safari have a...", and pointed me to the Safari release notes, but I don't see a mention of Chrome anywhere in that article |
| 16:07 | <annevk> | domfarolino: oops, Fx and Safari 🙂 |
| 16:08 | <domfarolino> | annevk: I see. For Fx at least, is that policy only used for document.referrer redaction, or elsewhere too? |
| 16:09 | <annevk> | domfarolino: I'm not sure, though I did just suggest internally that we should be consistent across .referrer and Referer |
| 16:09 | <domfarolino> | annevk: Gotcha, makes sense |
| 16:09 | <annevk> | (ideally) |
| 16:55 | <Domenic> | Alright, let's merge COEP! |
| 17:03 | <annevk> | I suspect I won't get to cross-origin isolated this week, but maybe |
| 17:26 | <Domenic> | https://github.com/WICG/origin-isolation/issues/31 would also be appreciated |
| 17:26 | <annevk> | Domenic: I have no strong opinions |
| 17:27 | <Domenic> | annevk: me neither. Charlie's seem pretty strong, so I guess maybe we should move back to (1) |
| 17:27 | <annevk> | Domenic: seems fine, also no good ideas on naming |
| 17:27 | <annevk> | gotta go |
| 18:00 | <Krinkle> | Hm.. is OI (or COI) another addition to the list of origin-related accronyms? (CORS, COEP, COOP) |
| 18:00 | <Krinkle> | (or COPI, for cross-origina process isolation) |
| 18:01 | <Krinkle> | (or COACI, given the term agent cluster is used in spec language) |
| 18:03 | <Krinkle> | maybe the specs can be merged into larger one, we can call it Big O. |
| 18:17 | <Domenic> | If you feel a need to abbreviate "origin isolation", I would suggest "OI" |
| 23:17 | <MikeSmith> | TabAtkins: r? on https://github.com/tabatkins/bikeshed/pull/1711 when you got time |