[sans-io] HTTP/2: reset half-closed streams on error

This commit is contained in:
Maximilian Hils 2020-12-10 14:28:45 +01:00
parent 09d9c608a4
commit 8f516bfd81
4 changed files with 39 additions and 13 deletions

View File

@ -11,11 +11,6 @@ class HttpEvent(events.Event):
# we need stream ids on every event to avoid race conditions # we need stream ids on every event to avoid race conditions
stream_id: StreamId stream_id: StreamId
def __repr__(self) -> str:
x = self.__dict__.copy()
x.pop("stream_id")
return f"{type(self).__name__}({repr(x) if x else ''})"
class HttpConnection(layer.Layer): class HttpConnection(layer.Layer):
conn: Connection conn: Connection

View File

@ -107,7 +107,8 @@ class Http1Connection(HttpConnection, metaclass=abc.ABCMeta):
# see https://github.com/httpwg/http-core/issues/22 # see https://github.com/httpwg/http-core/issues/22
if event.connection.state is not ConnectionState.CLOSED: if event.connection.state is not ConnectionState.CLOSED:
yield commands.CloseConnection(event.connection) yield commands.CloseConnection(event.connection)
yield ReceiveHttp(self.ReceiveProtocolError(self.stream_id, f"Client disconnected.")) yield ReceiveHttp(self.ReceiveProtocolError(self.stream_id, f"Client disconnected.",
code=status_codes.CLIENT_CLOSED_REQUEST))
else: # pragma: no cover else: # pragma: no cover
raise AssertionError(f"Unexpected event: {event}") raise AssertionError(f"Unexpected event: {event}")

View File

@ -14,7 +14,7 @@ import h2.utilities
from mitmproxy import http from mitmproxy import http
from mitmproxy.net import http as net_http from mitmproxy.net import http as net_http
from mitmproxy.net.http import url from mitmproxy.net.http import url, status_codes
from mitmproxy.utils import human from mitmproxy.utils import human
from . import RequestData, RequestEndOfMessage, RequestHeaders, RequestProtocolError, ResponseData, \ from . import RequestData, RequestEndOfMessage, RequestHeaders, RequestProtocolError, ResponseData, \
ResponseEndOfMessage, ResponseHeaders, ResponseProtocolError ResponseEndOfMessage, ResponseHeaders, ResponseProtocolError
@ -86,9 +86,11 @@ class Http2Connection(HttpConnection):
self.streams.pop(event.stream_id, None) self.streams.pop(event.stream_id, None)
elif isinstance(event, self.SendProtocolError): elif isinstance(event, self.SendProtocolError):
stream = self.h2_conn.streams.get(event.stream_id) stream = self.h2_conn.streams.get(event.stream_id)
if stream.state_machine.state not in (h2.stream.StreamState.HALF_CLOSED_LOCAL, if stream.state_machine.state is not h2.stream.StreamState.CLOSED:
h2.stream.StreamState.CLOSED): code = {
self.h2_conn.reset_stream(event.stream_id, h2.errors.ErrorCodes.INTERNAL_ERROR) status_codes.CLIENT_CLOSED_REQUEST: h2.errors.ErrorCodes.CANCEL,
}.get(event.code, h2.errors.ErrorCodes.INTERNAL_ERROR)
self.h2_conn.reset_stream(event.stream_id, code)
if self.is_closed(event.stream_id): if self.is_closed(event.stream_id):
self.streams.pop(event.stream_id, None) self.streams.pop(event.stream_id, None)
else: else:
@ -150,7 +152,11 @@ class Http2Connection(HttpConnection):
err_str = h2.errors.ErrorCodes(event.error_code).name err_str = h2.errors.ErrorCodes(event.error_code).name
except ValueError: except ValueError:
err_str = str(event.error_code) err_str = str(event.error_code)
yield ReceiveHttp(self.ReceiveProtocolError(event.stream_id, f"stream reset by client ({err_str})")) err_code = {
h2.errors.ErrorCodes.CANCEL: status_codes.CLIENT_CLOSED_REQUEST,
}.get(event.error_code, self.ReceiveProtocolError.code)
yield ReceiveHttp(self.ReceiveProtocolError(event.stream_id, f"stream reset by client ({err_str})",
code=err_code))
self.streams.pop(event.stream_id) self.streams.pop(event.stream_id)
else: else:
pass # We don't track priority frames which could be followed by a stream reset here. pass # We don't track priority frames which could be followed by a stream reset here.

View File

@ -8,13 +8,13 @@ from h2.errors import ErrorCodes
from mitmproxy.flow import Error from mitmproxy.flow import Error
from mitmproxy.http import HTTPFlow from mitmproxy.http import HTTPFlow
from mitmproxy.net.http import Headers from mitmproxy.net.http import Headers, Request, status_codes
from mitmproxy.proxy.protocol.http import HTTPMode from mitmproxy.proxy.protocol.http import HTTPMode
from mitmproxy.proxy2.commands import CloseConnection, OpenConnection, SendData from mitmproxy.proxy2.commands import CloseConnection, OpenConnection, SendData
from mitmproxy.proxy2.context import Context, Server from mitmproxy.proxy2.context import Context, Server
from mitmproxy.proxy2.events import ConnectionClosed, DataReceived from mitmproxy.proxy2.events import ConnectionClosed, DataReceived
from mitmproxy.proxy2.layers import http from mitmproxy.proxy2.layers import http
from mitmproxy.proxy2.layers.http._http2 import split_pseudo_headers from mitmproxy.proxy2.layers.http._http2 import split_pseudo_headers, Http2Client
from test.mitmproxy.proxy2.layers.http.hyper_h2_test_helpers import FrameFactory from test.mitmproxy.proxy2.layers.http.hyper_h2_test_helpers import FrameFactory
from test.mitmproxy.proxy2.tutils import Placeholder, Playbook, reply from test.mitmproxy.proxy2.tutils import Placeholder, Playbook, reply
@ -504,3 +504,27 @@ def test_kill_stream(tctx):
hyperframe.frame.SettingsFrame, hyperframe.frame.SettingsFrame,
hyperframe.frame.HeadersFrame, hyperframe.frame.HeadersFrame,
] ]
class TestClient:
def test_no_data_on_closed_stream(self, tctx):
frame_factory = FrameFactory()
req = Request.make("GET", "http://example.com/")
resp = {
":status" : 200
}
assert (
Playbook(Http2Client(tctx))
<< SendData(tctx.server, Placeholder(bytes)) # preamble + initial settings frame
>> DataReceived(tctx.server, frame_factory.build_settings_frame({}, ack=True).serialize())
>> http.RequestHeaders(1, req, end_stream=True)
<< SendData(tctx.server, b"\x00\x00\x06\x01\x05\x00\x00\x00\x01\x82\x86\x84\\\x81\x07")
>> http.RequestEndOfMessage(1)
>> DataReceived(tctx.server, frame_factory.build_headers_frame(resp).serialize())
<< http.ReceiveHttp(Placeholder(http.ResponseHeaders))
>> http.RequestProtocolError(1, "cancelled", code=status_codes.CLIENT_CLOSED_REQUEST)
<< SendData(tctx.server, frame_factory.build_rst_stream_frame(1, ErrorCodes.CANCEL).serialize())
>> DataReceived(tctx.server, frame_factory.build_data_frame(b"foo").serialize())
<< SendData(tctx.server, frame_factory.build_rst_stream_frame(1, ErrorCodes.STREAM_CLOSED).serialize())
# important: no ResponseData event here!
)