| 10:03 | <nox> | Where in the Fetch spec does it say to set the Origin header for WebSocket requests? |
| 10:19 | <MikeSmith> | nox: where it mentions ws: and wss: protocol? |
| 10:19 | <MikeSmith> | I thought it does there |
| 10:19 | MikeSmith | loosk |
| 10:19 | <nox> | MikeSmith: I don't see that. |
| 10:20 | <nox> | MikeSmith: All I see is "If the CORS flag is set or httpRequest’s method is neither `GET` nor `HEAD`, then append `Origin`/httpRequest’s origin, serialized and UTF-8 encoded, to httpRequest’s header list." |
| 10:20 | <nox> | Step 11 of http-network-or-cache-fetch, |
| 10:20 | <nox> | but AFAIK with a WebSocket request the CORS flag is unset. |
| 10:21 | <MikeSmith> | hmm then maybe in the HTML spec, where it calls the fetch algorithm for WebSocket requests |
| 10:21 | <nox> | MikeSmith: https://html.spec.whatwg.org/multipage/comms.html#dom-websocket Doesn't seem like it. |
| 10:22 | <MikeSmith> | right |
| 10:22 | <MikeSmith> | I see that just references https://fetch.spec.whatwg.org/#concept-websocket-establish |
| 10:25 | <nox> | MikeSmith: Seems like the step I mentioned should have an additional condition, |
| 10:25 | <nox> | MikeSmith: 'or request's mode is "websocket"' |
| 10:30 | <MikeSmith> | hmm yeah |
| 10:36 | <nox> | MikeSmith: When going through establish a ws connection, |
| 10:36 | <nox> | MikeSmith: can the response returned by basic fetch ever be a filtered response? |
| 10:36 | <nox> | In step 14 of https://fetch.spec.whatwg.org/#concept-main-fetch, that is. |
| 10:37 | MikeSmith | loosk |
| 10:41 | <MikeSmith> | nox: if the response tainting is the default “basic” it can, right? |
| 10:41 | <nox> | MikeSmith: Maybe? |
| 10:41 | <botie> | i heard Maybe was botie taking notes of who are going to TPAC |
| 10:41 | <MikeSmith> | request’s mode is "navigate" or "websocket" |
| 10:41 | <MikeSmith> | Set request’s response tainting to "basic". |
| 10:42 | <annevk> | nox: if even same-origin WebSocket includes it I agree that we need a change there |
| 10:42 | <nox> | MikeSmith: Yeah but I'm not sure it matters in the particular case of websocket. |
| 10:42 | <MikeSmith> | Set response to the following filtered response with response as its internal response, depending on request’s response tainting: |
| 10:42 | <annevk> | nox: I guess the WebSocket handshake request uses GET? |
| 10:42 | <MikeSmith> | "basic" |
| 10:42 | <MikeSmith> | basic filtered response |
| 10:42 | <nox> | annevk: It does. |
| 10:42 | <MikeSmith> | yeah it does |
| 10:43 | <annevk> | nox: I think you're right then that we need to include mode is websocket |
| 10:43 | <MikeSmith> | it does use GET because it defaults to that, right? |
| 10:44 | <nox> | MikeSmith: Oh I see the step you pasted, |
| 10:44 | <nox> | MikeSmith: but the previous step to the one which you pasted is "If response is not a network error and response is not a filtered response, then run these substeps:", |
| 10:45 | <nox> | MikeSmith: can HTTP fetch itself return a filtered response for ws? |
| 10:45 | <MikeSmith> | aha |
| 10:45 | <MikeSmith> | nox: it cannot there as far as I can see |
| 10:48 | <nox> | MikeSmith: Thanks! |
| 10:50 | <MikeSmith> | cheers |
| 10:50 | <MikeSmith> | the more I read the Fetch spec the more I think there’s no way to understand it without implementing it |
| 10:50 | <MikeSmith> | which I’m not doing |
| 10:51 | <MikeSmith> | but which I guess you are |
| 10:52 | <nox> | MikeSmith: It's worse than that. |
| 10:52 | <nox> | MikeSmith: Our Fetch impl currently doesn't allow us to use these algos from the websocket case, |
| 10:52 | <nox> | MikeSmith: and the three existing Rust impls of websocket don't play well with that spec in general, |
| 10:53 | <nox> | so I'm reimplementing these algos special-cased for ws, and doing ws handshake myself from Servo's code directly. |
| 10:53 | <nox> | The unfiltered internal response does not matter in the ws case, right? |
| 10:54 | <nox> | Actually, does filtering these responses matter at all? |
| 10:54 | <nox> | The response is never exposed to the Web for websockets, right? |
| 10:55 | <MikeSmith> | I guess not unless some other spec says it is? |
| 10:55 | <nox> | MikeSmith: You devil. |
| 10:55 | <MikeSmith> | which if so should only be the HTML spec I’d think |
| 10:55 | <MikeSmith> | heh |
| 10:55 | <MikeSmith> | *snort* |
| 10:57 | <MikeSmith> | the Fetch spec should be used on people as a test of how must perseverance they have |
| 10:57 | <nox> | Heh. |
| 11:36 | <nox> | MikeSmith, annevk: https://github.com/whatwg/fetch/pull/518 |
| 11:48 | <annevk> | reviewed |
| 11:49 | <nox> | annevk: Amended. |
| 11:49 | <annevk> | nox: do you know if we have tests? |
| 11:50 | <nox> | annevk: Absolutely no idea, but the Origin header seems core to WS to me, so it wouldn't surprise me if we do. |
| 13:19 | <nox> | annevk, MikeSmith: Shouldn't step 22.5.2. be avoided for websocket stuff? |
| 13:19 | <nox> | annevk, MikeSmith: Why should even the response be cached at all if the cache mode is no-store? |
| 13:20 | <nox> | Ah nevermind, that's covered by the mentioned RFC... |
| 13:20 | <annevk> | nox: might be a recent regression, mnot revamped the cache bits |
| 13:47 | smaug | wants "right after rAF" callback |
| 13:49 | <annevk> | smaug: I think you filed a bug for that at some point, it hasn't gotten fixed thus far |
| 13:49 | <smaug> | I did |
| 13:50 | <smaug> | it just occurs every now and then when I write some benchmarking like code that I end up doing rAF(function(setTimeout(...))) |
| 13:50 | <smaug> | hmm, perhaps I could use requestIdleCallback |
| 14:17 | <nox> | annevk: What about the last bullet of step 5 of https://fetch.spec.whatwg.org/#concept-http-network-fetch? |
| 14:18 | <nox> | annevk: Is the server allowed to return some non-101 1xx response, and then the 101 one, for WebSocket? |
| 14:21 | <annevk> | nox: per the new definition yes, but this wasn't tested |
| 14:21 | <annevk> | nox: not sure what's desired |
| 14:22 | <annevk> | nox: making the handshake behave as much as the rest of the networking stack as possible probably is, so ignoring seems better? |
| 14:42 | <annevk> | zcorpan: per grep websockets/Create-Secure-blocked-port.htm is the only resource that makes use of websocket.sub.js |
| 14:42 | <zcorpan> | annevk: huh |
| 14:42 | <annevk> | zcorpan: I think I'll push another fixup that basically inlines the creation of that function |
| 14:42 | <annevk> | zcorpan: sorry, for CreateWebSocketWithBlockedPort |
| 14:43 | <annevk> | zcorpan: per "grep CreateWebSocketWithBlockedPort -r *" |
| 14:43 | <zcorpan> | annevk: ah, yes indeed |
| 14:44 | <zcorpan> | need .sub.html on test then to get the port that should work |
| 14:48 | <annevk> | ah right, skipped inlining for now and just made it return |
| 14:48 | <annevk> | websocket.sub.js already returns sometimes |
| 14:49 | <annevk> | mkwst: ping for https://github.com/whatwg/html/issues/2460 |
| 14:49 | <botie> | will do |
| 14:56 | <annevk> | domfarolino: filed an Edge issue successfully just now btw |
| 14:59 | <domfarolino> | annevk: thanks for the heads up! Forgot to reply the other day, ended up using the twitter reporting which worked great |
| 15:03 | <mkwst> | annevk: Thanks! I've thrown it at folks smarter than me. |
| 15:08 | <nox> | annevk: In step 5 of https://fetch.spec.whatwg.org/#concept-http-network-fetch, |
| 15:08 | <nox> | it says 'If request’s header list contains `Transfer-Encoding`/`chunked` and response is transferred via HTTP/1.0 or older, then return a network error.', |
| 15:08 | <nox> | and then 'Transmit body for request', |
| 15:08 | <nox> | how can we know the response is transferred via HTTP/1.0 before even sending the body? |
| 15:11 | <annevk> | nox: it's possible in theory to get an early response, but I guess that should be part of the caveats somehow |
| 15:12 | <annevk> | nox: all of step 5 is sort of continuous in a way :/ |
| 15:12 | <annevk> | nox: main problem is that standards are not nicely layered |
| 15:12 | <nox> | annevk: Oh really! ;) |
| 15:48 | <annevk> | There really should be some kind of way to rebase where it adds a commit for rebase conflicts that makes the rebasing work and that can then be reviewed |
| 15:48 | <annevk> | Just rebasing and force pushing and no longer being sure about your entire review is really bad |
| 22:21 | <tobie> | Domenic: looking at https://github.com/heycam/webidl/pull/286 today hoping to be able to merge it, I'm left wondering whether the right decision wouldn't be to have a separate syntax for type modifiers vs. extended attributes. |
| 22:23 | <tobie> | Domenic: [clamp] on a dictionary member clearly addresses the type, while the hinted at [SecureContext] on the same member, clearly addresses the member. |
| 22:33 | <tobie> | Domenic: mmm. letting it sink in, it seems these distinctions will be completely opaque unless you're attempting to specify extended attributes. |