Skip to content
This repository has been archived by the owner on May 17, 2019. It is now read-only.

HTTP/2 Support #133

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

HTTP/2 Support #133

wants to merge 30 commits into from

Conversation

sorpaas
Copy link

@sorpaas sorpaas commented Aug 4, 2016

This pull request adds HTTP/2 support for Requests.jl. Help on testing and code review are highly appreciated!

To use it, you need to checkout master of HPack.jl (header compression implementation), HTTP2.jl (HTTP/2 protocol implementation) and MbedTLS.jl.

For a simple test, install nghttp. Run in shell nghttpd --verbose --no-tls --hexdump 9000 to start a server and then in Julia:

using Requests
res = get("http://127.0.0.1:9000", http2=true)

@show res.headers

To test HTTPS, create a test certificate and key. Start the server by nghttpd --verbose --hexdump 9000 test.key test.crt and then in Julia:

using Requests
res = get("https://127.0.0.1:9000", tls_conf=Requests.TLS_NOVERIFY, http2=true)

@show res.headers

You can also test it against a server in the real world. Usually, wild HTTP/2 servers require HTTPS support.

using Requests
res = get("https://www.google.com", http2=true)

@sorpaas
Copy link
Author

sorpaas commented Aug 8, 2016

Updated MbedTLS and HTTP2's version and tests should pass now.

@sorpaas sorpaas changed the title WIP: HTTP/2 Support HTTP/2 Support Aug 12, 2016
@malmaud
Copy link
Contributor

malmaud commented Oct 4, 2016

Hey @sorpaas, hoping to merge this soon. I tried rerunning the Travis tests and there seems to be a sporadic failure. Any idea what's going on?

@sorpaas
Copy link
Author

sorpaas commented Oct 5, 2016

I still cannot reproduce that locally even now I got a MacBook. On Travis it is still a segmentation fault only on nightly:

signal (11): Segmentation fault: 11
while loading /Users/travis/.julia/v0.6/Requests/test/runtests.jl, in expression starting on line 189
strlen at /usr/lib/system/libsystem_c.dylib (unknown line)

Also, any ways to run the Travis build locally on OS X? I thought of Docker but it uses a VM so it's still Linux.

@sorpaas
Copy link
Author

sorpaas commented Oct 5, 2016

Also, the line 189 (where it fails) actually doesn't relate to HTTP2:

@test statuscode(get("https://httpbin.org")) == 200

@sorpaas
Copy link
Author

sorpaas commented Oct 5, 2016

@malmaud Tests now passed on nightly! :)

It turns out to be a problem with MbedTLS. Failing back to version 0.2.1 now works.

@sorpaas sorpaas closed this Oct 5, 2016
@sorpaas sorpaas reopened this Oct 5, 2016
@sorpaas sorpaas closed this Oct 5, 2016
@sorpaas sorpaas reopened this Oct 5, 2016
@sorpaas sorpaas closed this Oct 5, 2016
@sorpaas sorpaas reopened this Oct 5, 2016
@sorpaas
Copy link
Author

sorpaas commented Oct 5, 2016

It fails again.... :(

I guess the segmentation fault is somehow related to MbedTLS.

joshbode and others added 2 commits October 7, 2016 17:19
- `gzip_data`: Zips provided data and adds "Content-Encoding: gzip" header
- `compressed`: Adds "Accept-Encoding: gzip, deflate" header to request
Additional support for GZipped data
@codecov-io
Copy link

codecov-io commented Nov 19, 2016

Codecov Report

Merging #133 into master will increase coverage by 2.23%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
+ Coverage   68.68%   70.92%   +2.23%     
==========================================
  Files           4        4              
  Lines         511      626     +115     
==========================================
+ Hits          351      444      +93     
- Misses        160      182      +22
Impacted Files Coverage Δ
src/Requests.jl 65.03% <56.41%> (+6.65%) ⬆️
src/streaming.jl 78.35% <76.47%> (+4.19%) ⬆️
src/parsing.jl 90.32% <0%> (-1.08%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ea3eca...56d5097. Read the comment docs.

@musm
Copy link

musm commented Mar 20, 2017

try again ci?

@sorpaas
Copy link
Author

sorpaas commented Mar 20, 2017

Ah, I'm really sorry that I haven't got this merged. I'll try to re-base the code and do code improvements. (Might take a few days due to my workload recently ...)

@malmaud
Copy link
Contributor

malmaud commented Mar 20, 2017 via email

@musm
Copy link

musm commented Apr 3, 2017

Note I am having the same failure on the http post test at #151
it only recently started failing and I believe it's unrelated

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.