Allow depends_on and weight settings in send_headers #229

Activity

1. Lukasa commented on May 21, 2016

   

   Lukasa

   on May 21, 2016

   Member

   More actions

   So this is two separate concerns. Let's make them real explicit. =)

   1. Emitting priority information. Yes, the API currently has no support for it, and we should definitely add it. We'll want to add two things: extend the send_headers callable and add a new prioritize (or similar) callable.
   2. PriorityUpdated and *Received events. In general I'm happy to keep these as separate events, but I'm acutely aware that it makes it very difficult to reproduce the original frame exactly. I'd like to get an idea of how important that is for mitmproxy, because right now hyper-h2 already doesn't guarantee that that is possible (e.g. we hide CONTINUATION frames from the user). If it's important, we can consider keeping hold of the frame object and attaching it to the event itself, which might make this easier.
2. Kriechi commented on May 22, 2016

   

   Kriechi

   on May 22, 2016

   MemberAuthor

   More actions

   Ideally we (@mitmproxy) would like to reproduce frames as accurate as possible.

   I'm not sure if there is a semantic difference between HEADERS with priority information, and HEADERS+PRIORITY frames. But I would rather reflect a single incoming HEADERS frame with the same frame sending out - and not having to generate two frames...
   Adding code to implement concern 1 (as per your list) would be sufficient for as at the moment.

   Nevertheless, it would be nice to distinguish between priority information from a HEADERS frame (we would call send_headers with additional arguments), and an independently received PRIORITY frame (we would need to call h2_conn.prioritize).

   For CONTINUATION frames this might be a bit tricky, but since we already mirror MAX_FRAME_SIZE from client to server, the header_block_fragment splitting should occur at the same length - producing the same amount of frames - at least that's what we are hoping for.
3. Lukasa commented on May 23, 2016

   

   Lukasa

   on May 23, 2016

   Member

   More actions

   For CONTINUATION frames this might be a bit tricky, but since we already mirror MAX_FRAME_SIZE from client to server, the header_block_fragment splitting should occur at the same length - producing the same amount of frames - at least that's what we are hoping for.

   I don't think that happens: I've definitely seen Chrome emit CONTINUATION frames when emitting header blocks well below the MAX_FRAME_SIZE that Chrome sets. I'd have to go back and double-check, but I'm pretty confident Chrome does weird stuff here.

   Regardless, my reading is that part 1 of this is important to you and we should shoot to do it soon, but that part 2 is worth taking longer to think over.

   While we're here, you may want to look at @rbtcollins' suggestions in #228. This includes the idea of showing "simultaneous" events by emitting a frozenset containing both of them at once. If we adopted that API change you actually would be able to tell the difference between HEADERS then PRIORITY and HEADERS with PRIORITY extension, because the first would emit two separate events and the second would emit a frozenset with both. How does that idea sound?
4. Kriechi commented on May 25, 2016

   

   Kriechi

   on May 25, 2016

   Last edited by Kriechi

   MemberAuthor

   More actions

   Sounds all good! 👍

   If we continue the path to "simultaneous events" (or something similar), we could add the information in how many chunks the headers were split ot RequestReceived and ResponseReceived, e.g. we could add a list with the header_block_fragment sized, which translates into how many CONTINUATION frames have been received.

   Of the top of my head I think events can simply be grouped by their associated stream-id.
5. Lukasa

   closed this as completedin #233on May 29, 2016