Skip to content

Reject response header blocks that don’t include the :status field #290

Closed

Description

@alexwlchan

Quoting from RFC 7540 section 8.1.2.4:

For HTTP/2 responses, a single ":status" pseudo-header field is
defined that carries the HTTP status code field (see [RFC7231],
Section 6). This pseudo-header field MUST be included in all
responses; otherwise, the response is malformed (Section 8.1.2.6).

This would mean adding another check in utilities.py to cover the possibility that this pseudo-header field is missing, and adding some new test cases in test_invalid_headers.py.

Activity

  1. alexwlchan commented on Aug 19, 2016

    @alexwlchan
    ContributorAuthor

    @Lukasa I’m not going to do this myself, because (1) I don’t have the time right now, and (2) I think this might be a good first patch for somebody else to take on.

    But I did have a brief look at it, and I think we might want another field on HeaderValidationFlags. Specifically – is this header block being sent or received?

    The sensible place (performance-wise) to do this check would be _reject_pseudo_header_fields – we’ve already built up a set of pseudo-header field names, so we can just check b':status' in seen_pseudo_header_fields, but I don’t think the current header validation flags give us enough information to know if this is a response or not.

  2. alexwlchan commented on Aug 19, 2016

    @alexwlchan
    ContributorAuthor

    (Buttons, how do they work?!)

  3. Lukasa commented on Aug 20, 2016

    @Lukasa
    Member

    Thanks for raising this up @alexwlchan! @python-hyper/contributors, if any of you feel like picking this up or have someone in mind who needs a good start in OSS, please assign the issue to yourself and tag the person you want to do the work. If not, I'll also go looking to find a new OSS contributor.

  4. Lukasa commented on Aug 30, 2016

    @Lukasa
    Member

    Header validation should never strip a header. It may reject a header block (raise ProtocolError), but never strip a header. Do you have a test case that shows this behaviour?

    As to in HeaderValidationFlags, I think the flag we need is "is_response_header", which is set to True when we're sending a response header block.

  5. Lukasa commented on Aug 31, 2016

    @Lukasa
    Member

    See my notes on the commit. =)

  6. Lukasa commented on Sep 6, 2016

    @Lukasa
    Member

    Sure thing @aidylewis, that's no problem at all. This can go back on the pile. =)

  7. sunu commented on Sep 6, 2016

    @sunu
    Member

    I can pick this up if it's okay.

  8. Lukasa commented on Sep 6, 2016

    @Lukasa
    Member

    Fine by me!

  9. added a commit that references this issue on Sep 20, 2016
  10. Lukasa commented on Sep 20, 2016

    @Lukasa
    Member

    Resolved by #311.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions