mike820324 / microProxy

A http/https interceptor written in pure python.
MIT License
17 stars 3 forks source link

[WIP] Sync HTTP2 stream id between source and destination. #244

Closed mike820324 closed 6 years ago

mike820324 commented 7 years ago

This PR change the following features.

Slightly changed the log format :

Put the direction(source/destination) in front of the message.

Upgrade Hyper-h2 to 2.5.1 :

Since hyper-h2 2.5.0, it will check whether the header field contains Host or not. As a result, update some unittests according to this.

Sync stream id between source and destination:

The original code will drop some frames(especially the priority update frame. I forgot the reason why we drop these) and hence causing the stream id between source and destination unsync. This PR remove some of the code in layer/http2.py and just send the same stream id to the other endpoint.

P.S @chhsiao90 can you kindly remind me that why we added this code. Orz

Pending some frames:

Due to the fact that we will pending the HEADER frame until we receive the StreamEnded event, we need to pending some of the frames that we receive until our HEADER frame is sent.

Prevent duplicate priority frame:

When hyper-h2 receive HEADER frame with priority updated, it will actually generate two events which are

Based on our current implementation,

As a result, I have added a simple logic on top of the receive_data method to check any duplicate PriorityUpdate event.

TODO:

This is still WIP. @chhsiao90 please help me review this part since you are expert in this category. Any advice is thankful.

chhsiao90 commented 7 years ago

Wow, finally, you remembered this repository!

chhsiao90 commented 7 years ago

Simply ask, you drop the src <-> dest stream id mapping, and just straight forward with same stream id ? I think that would be ok!

mike820324 commented 7 years ago

Yes, I have drop the steam id mapping. Do you remember why we add this mapping code, I believe there used to be a bug related to this, but I forgot. :(

chhsiao90 commented 7 years ago

I remembered that legacy hyper-h2 didn't support for jumping the stream id.

mike820324 commented 7 years ago

Oh, thanks for reminder. So it should be okay to remove the mappings part of code?

chhsiao90 commented 7 years ago

If the test is ok, I think it's fine to remove!

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.7%) to 86.109% when pulling 81cf5d3253dd277a74db884db72a4738d8bdd0fe on http2-streamid-sync into eb8e14069189c6ff11a4953daacdb1bec8fa5db0 on master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.7%) to 86.109% when pulling 81cf5d3253dd277a74db884db72a4738d8bdd0fe on http2-streamid-sync into eb8e14069189c6ff11a4953daacdb1bec8fa5db0 on master.

chhsiao90 commented 7 years ago

That me simply describe what I understand about the pending event and header receive event.

You introduce pending event is for the purpose to delay the event sending to the destination stream of WINDOW_UPDATE, PRIORITY, RESET that the stream still in idle state. And the header receive is just fulfill the pending event functionality.

But why we need this? Is the hyper-h2 restriction?