Skip to content
This repository has been archived by the owner on Jan 13, 2021. It is now read-only.

Hyper not stripping Connection header field #291

Open
persiaAziz-zz opened this issue Oct 24, 2016 · 15 comments
Open

Hyper not stripping Connection header field #291

persiaAziz-zz opened this issue Oct 24, 2016 · 15 comments

Comments

@persiaAziz-zz
Copy link

Hyper sends TCP-FIN to the latest build of apache traffic server right after sending the request data if I mention 'Connection':'Keep-Alive' in my request header.

@Lukasa
Copy link
Member

Lukasa commented Oct 25, 2016

@persiaAziz I'm afraid I'll need a little bit more information. Can you provide the code you're using please?

@persiaAziz-zz
Copy link
Author

persiaAziz-zz commented Oct 25, 2016

import json
from hyper import HTTPConnection
import hyper
import argparse

def getResponseString(response):
    string = "HTTP/2 {0}\r\n".format(response.status)
    string+='date: '+response.headers.get('date')[0].decode('utf-8')+"\r\n"
    string+='server: '+response.headers.get('Server')[0].decode('utf-8')+"\r\n"
    return string

def makerequest(port):
    hyper.tls._context = hyper.tls.init_context()
    hyper.tls._context.check_hostname = False
    hyper.tls._context.verify_mode = hyper.compat.ssl.CERT_NONE

    conn = HTTPConnection('localhost:443'.format(port), secure=True)

    sites={'www.example.com'}
    #print("port---------",port)
    responses = []
    request_ids = []
    hdr={'Host':'www.example.com','Connection':'Keep-Alive'}
    for site in sites:
            request_id = conn.request('GET',url=site,headers=hdr)
            request_ids.append(request_id)

        # get responses
    for req_id in request_ids:
        response = conn.get_response(req_id)    
        print(getResponseString(response))


def main():
    parser = argparse.ArgumentParser()
    parser.add_argument("--port","-p",
                        type=int,                        
                        help="Port to use")
    args=parser.parse_args()
    makerequest(args.port)

if __name__ == '__main__':
    main()

@persiaAziz-zz
Copy link
Author

You can reproduce the issue with the above code. It is the Connection field in the header that causes the issue.

@Lukasa
Copy link
Member

Lukasa commented Oct 25, 2016

@persiaAziz Do you get an exception in this code? I suspect you will.

@persiaAziz-zz
Copy link
Author

Yes I do @Lukasa it tells me 'stream forcefully closed' as I try to read the response

@Lukasa
Copy link
Member

Lukasa commented Oct 25, 2016

Can you print the full stdout?

@persiaAziz-zz
Copy link
Author

persiaAziz-zz commented Oct 25, 2016

`python3.5 h2client.py -p 443

Traceback (most recent call last):
File "h2client.py", line 46, in
main()
File "h2client.py", line 43, in main
makerequest(args.port)
File "h2client.py", line 31, in makerequest
response = conn.get_response(req_id)
File "/usr/local/lib/python3.5/dist-packages/hyper/common/connection.py", line 129, in get_response
return self._conn.get_response(_args, *_kwargs)
File "/usr/local/lib/python3.5/dist-packages/hyper/http20/connection.py", line 312, in get_response
return HTTP20Response(stream.getheaders(), stream)
File "/usr/local/lib/python3.5/dist-packages/hyper/http20/stream.py", line 230, in getheaders
self._recv_cb(stream_id=self.stream_id)
File "/usr/local/lib/python3.5/dist-packages/hyper/http20/connection.py", line 768, in _recv_cb
self._get_stream(stream_id)
File "/usr/local/lib/python3.5/dist-packages/hyper/http20/connection.py", line 289, in _get_stream
raise StreamResetError("Stream forcefully closed")
hyper.http20.exceptions.StreamResetError: Stream forcefully closed`

@Lukasa
Copy link
Member

Lukasa commented Oct 25, 2016

Hrm. Can you run pip install -U h2 and try again please? I'm interested to see if you can get a different exception.

@persiaAziz-zz
Copy link
Author

hmm now It actually complains about that field

Traceback (most recent call last): File "h2client.py", line 46, in <module> main() File "h2client.py", line 43, in main makerequest(args.port) File "h2client.py", line 26, in makerequest request_id = conn.request('GET',url=site,headers=hdr) File "/usr/local/lib/python3.5/dist-packages/hyper/common/connection.py", line 121, in request method=method, url=url, body=body, headers=headers File "/usr/local/lib/python3.5/dist-packages/hyper/http20/connection.py", line 281, in request self.endheaders(message_body=body, final=True, stream_id=stream_id) File "/usr/local/lib/python3.5/dist-packages/hyper/http20/connection.py", line 555, in endheaders stream.send_headers(headers_only) File "/usr/local/lib/python3.5/dist-packages/hyper/http20/stream.py", line 98, in send_headers conn.send_headers(self.stream_id, headers, end_stream) File "/usr/local/lib/python3.5/dist-packages/h2/connection.py", line 803, in send_headers headers, self.encoder, end_stream File "/usr/local/lib/python3.5/dist-packages/h2/stream.py", line 788, in send_headers headers, encoder, hf, hdr_validation_flags File "/usr/local/lib/python3.5/dist-packages/h2/stream.py", line 1119, in _build_headers_frames encoded_headers = encoder.encode(headers) File "/usr/local/lib/python3.5/dist-packages/hpack/hpack.py", line 229, in encode for header in headers: File "/usr/local/lib/python3.5/dist-packages/h2/utilities.py", line 368, in _validate_host_authority_header for header in headers: File "/usr/local/lib/python3.5/dist-packages/h2/utilities.py", line 302, in _reject_pseudo_header_fields for header in headers: File "/usr/local/lib/python3.5/dist-packages/h2/utilities.py", line 274, in _reject_connection_header "Connection-specific header field present: %s." % header[0] h2.exceptions.ProtocolError: Connection-specific header field present: b'connection'.

@Lukasa
Copy link
Member

Lukasa commented Oct 25, 2016

Yup, that's what I thought.

So, the Connection header field is not allowed in HTTP/2. You should find this problem resolved if you remove it from your request. However, I'm going to consider this an open bug: hyper should strip it automatically anyway.

@Lukasa Lukasa changed the title Hyper sending FIN Hyper not stripping Connection header field Oct 25, 2016
@nateprewitt
Copy link
Member

Just a bit more information on this. I upgraded to h2==2.5.0 today and my code using the HTTP20Adapter with requests stopped working. I went back and tested the current example in the hyper docs and it hits this issue as well. This is because a 'Connection' headers appears in Request's default headers list.

I was originally going to suggest patching the adapter, but after reading through this it looks like it's a somewhat more pervasive issue. I believe what you're suggesting is stripping the 'Connection' header on all HTTP/2 connections if encountered, rather than raising an exception. Is that correct @Lukasa? I'd be willing to take a swing at this if no one else has started work on it.

@Lukasa
Copy link
Member

Lukasa commented Nov 4, 2016

That's exactly what I'm suggesting. I'm happy for you to take a swing at it.

@nateprewitt
Copy link
Member

This should now be resolved python-hyper/h2#382 on the master branch and available in hyper-h2's next release (2.6.0).

@hupantingxue
Copy link

How to work around it?

@Lukasa
Copy link
Member

Lukasa commented Jan 18, 2017

@hupantingxue The easiest thing to do is just to install from the development branch for now.

elipapa added a commit to opentargets-archive/opentargets-py that referenced this issue May 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants