Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions h2/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,13 @@ def _reject_pseudo_header_fields(headers, hdr_validation_flags):

yield header

# Pseudo-header fields MUST NOT appear in trailers - RFC 7540 § 8.1.2.1
if hdr_validation_flags.is_trailer and seen_pseudo_header_fields:
raise ProtocolError(
"Received pseudo-header in trailer %s" %
seen_pseudo_header_fields
)


def _validate_host_authority_header(headers):
"""
Expand Down
4 changes: 2 additions & 2 deletions test/test_complex_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ def test_correctly_count_server_streams(self, frame_factory):
assert c.open_outbound_streams == expected_outbound_streams

# Pushed streams can only be closed remotely.
f = frame_factory.build_headers_frame(
f = frame_factory.build_data_frame(
stream_id=stream_id+1,
headers=self.example_response_headers,
data=b'the content',
flags=['END_STREAM'],
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, seeing this change in place I now understand what you were getting at. Yes, there is nothing wrong with closing that stream using trailers, but the trailers that were being sent were not actually valid trailers. This change is better. =)

c.receive_data(f.serialize())
Expand Down
43 changes: 42 additions & 1 deletion test/test_invalid_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,37 @@ def test_transfer_encoding_trailers_is_valid(self, frame_factory):
request_event = events[0]
assert request_event.headers == headers

def test_pseudo_headers_rejected_in_trailer(self, frame_factory):
"""
Ensure we reject pseudo headers included in trailers
"""
trailers = [(':path', '/'), ('extra', 'value')]

c = h2.connection.H2Connection(client_side=False)
c.receive_data(frame_factory.preamble())
c.clear_outbound_data_buffer()

header_frame = frame_factory.build_headers_frame(
self.base_request_headers
)
trailer_frame = frame_factory.build_headers_frame(
trailers, flags=["END_STREAM"]
)
head = header_frame.serialize()
trailer = trailer_frame.serialize()

c.receive_data(head)
# Raise exception if pseudo header in trailer
with pytest.raises(h2.exceptions.ProtocolError) as e:
c.receive_data(trailer)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a quick confirmation that we're getting the right exception message here: this can save us from situations where this test stops failing for the reason we expect.

assert "pseudo-header in trailer" in str(e)

# Test appropriate response frame is generated
expected_frame = frame_factory.build_goaway_frame(
last_stream_id=1, error_code=h2.errors.ErrorCodes.PROTOCOL_ERROR
)
assert c.data_to_send() == expected_frame.serialize()


class TestSendingInvalidFrameSequences(object):
"""
Expand Down Expand Up @@ -217,6 +248,14 @@ class TestFilter(object):
]
]

hdr_validation_no_trailers = [
h2.utilities.HeaderValidationFlags(is_client, is_trailer)
for is_client, is_trailer in [
(True, False),
(False, False)
]
]

@pytest.mark.parametrize('validation_function', validation_functions)
@pytest.mark.parametrize('hdr_validation_flags', hdr_validation_combos)
@given(headers=HEADERS_STRATEGY)
Expand All @@ -241,7 +280,9 @@ def test_invalid_pseudo_headers(self, hdr_validation_flags):
h2.utilities.validate_headers(headers, hdr_validation_flags)

@pytest.mark.parametrize('validation_function', validation_functions)
@pytest.mark.parametrize('hdr_validation_flags', hdr_validation_combos)
@pytest.mark.parametrize(
'hdr_validation_flags', hdr_validation_no_trailers
)
def test_matching_authority_host_headers(self,
validation_function,
hdr_validation_flags):
Expand Down