HTTP.jl discards start of request

[Keno]

I thought I had filed this somewhere before, but we're seeing a pretty bad issue using HTTP.jl to interact with the GitHub API.

[HTTP - 2018-01-11T16:03:41.616]: received request on connection i=4
HTTP.Request:

POST / HTTP/1.1
X-Hub-Signature: sha1=bb3616be209738779a4bb3c4cada3c0bd5ec93da
X-Github-Event: pull_request
Host: [censored]
Accept: */*
Content-Length: 22453
Content-Type: application/json
User-Agent: GitHub-Hookshot/8f9d70f
X-Github-Delivery: fd84bdb0-f6e8-11e7-9a85-f9291adaee03
Content-Length: 9734

//api.github.com/repos/JuliaCI/BaseBenchmarks.jl/git/commits

Two things to note: The second Content-Length doesn't match the first and the shown body is obviously not the start of the body (which should start with { for valid JSON)

[Keno]

Looks like there's something wrong with FIFOBuffer (or how it's used). When the body is split over two trips through the parser, the second overwrites the first.

[DEBUG - HTTP./opt/pkgs/v0.6/HTTP/src/parser.jl:171  ]: this onbody 1
[DEBUG - HTTP./opt/pkgs/v0.6/HTTP/src/parser.jl:107  ]: onbody
[DEBUG - HTTP./opt/pkgs/v0.6/HTTP/src/parser.jl:107  ]: String(r.body) = {\"ref\":\"refs/heads/gh-pages\",\"before\":\"bf3c214b960c66c9d6d40c8086d1fc7446c88435\",\"after\":\"9187f29e75905a1312752493e87e89582a7fa2ff\",\"created\":false,\"deleted\":false,\"forced\":false,\"base_ref\":null,\"compare\":\"https://github.com/JuliaGPU/CUDAdrv.jl/compare/bf3c214b960c...9187f29e7590\",\"commits\":[{\"id\":\"9187f29e75905a1312752493e87e89582a7fa2ff\",\"tree_id\":\"3bd292a581a3494f321e03bd98bd79bb1edadc43\",\"distinct\":true,\"message\":\"build based on bbb3ae7\",\"timestamp\":\"2018-01-11T17:21:31Z\",\"url\":\"https://github.com/JuliaGPU/CUDAdrv.jl/commit/9187f29e75905a1312752493e87e89582a7fa2ff\",\"author\":{\"name\":\"autodocs\",\"email\":\"autodocs\"},\"committer\":{\"name\":\"autodocs\",\"email\":\"autodocs\"},\"added\":[\"v0.7.4/assets/arrow.svg\",\"v0.7.4/assets/documenter.css\",\"v0.7.4/assets/documenter.js\",\"v0.7.4/assets/search.js\",\"v0.7.4/index.html\",\"v0.7.4/lib/api.html\",\"v0.7.4/lib/array.html\",\"v0.7.4/man/usage.html\",\"v0.7.4/search.html\",\"v0.7.4/search_index.js\",\"v0.7.4/siteinfo.js\"],\"removed\":[],\"modified\":[\"release-0.7/lib/api.html\",\"release-0.7/lib/array.html\",\"release-0.7/search_index.js\",\"stable/lib/api.html\",\"stable/lib/array.html\",\"stable/search_index.js\",\"versions.js\"]}],\"head_commit\":{\"id\":\"9187f29e75905a1312752493e87e89582a7fa2ff\",\"tree_id\":\"3bd292a581a3494f321e03bd98bd79bb1edadc43\",\"distinct\":true,\"message\":\"build based on bbb3ae7\",\"timestamp\":\"2018-01-11T17:21:31Z\",\"url\":\"https://github.com/JuliaGPU/CUDAdrv.jl/commit/9187f29e75905a1312752493e87e89582a7fa2ff\",\"author\":{\"name\":\"autodocs\",\"email\":\"autodocs\"},\"committer\":{\"name\":\"autodocs\",\"email\":\"autodocs\"},\"added\":[\"v0.7.4/assets/arrow.svg\",\"v0.7.4/assets/documenter.css\",\"v0.7.4/assets/documenter.js\",\"v0.7.4/assets/search.js\",\"v0.7.4/index.html\",\"v0.7.4/lib/api.html\",\"v0.7.4/lib/array.html\",\"v0.7.4/man/usage.html\",\"v0.7.4/search.html\",\"v0.7.4/search_index.js\",\"v0.7.4/siteinfo.js\"],\"removed\":[],\"modified\":[\"release-0.7/lib/api.html\",\"release-0.7/lib/array.html\",\"release-0.7/search_index.js\",\"stable/lib/api.html\",\"stable/lib/array.html\",\"stable/search_index.js\",\"versions.js\"]},\"repository\":{\"id\":52348309,\"name\":\"CUDAdrv.jl\",\"full_name\":\"JuliaGPU/CUDAdrv.jl\",\"owner\":{\"name\":\"JuliaGPU\",\"email\":\"\",\"login\":\"JuliaGPU\",\"id\":7346142,\"avatar_url\":\"https://avatars2.githubusercontent.com/u/7346142?v=4\",\"gravatar_id\":\"\",\"url\":\"https://api.github.com/users/JuliaGPU\",\"html_url\":\"https://github.com/JuliaGPU\",\"followers_url\":\"https://api.github.com/users/JuliaGPU/followers\",\"following_url\":\"https://api.github.com/users/JuliaGPU/following{/other_user}\",\"gists_url\":\"https://api.github.com/users/JuliaGPU/gists{/gist_id}\",\"starred_url\":\"https://api.github.com/users/JuliaGPU/starred{/owner}{/repo}\",\"subscriptions_url\":\"https://api.github.com/users/JuliaGPU/subscriptions\",\"organizations_url\":\"https://api.github.com/users/JuliaGPU/orgs\",\"repos_url\":\"https://api.github.com/users/JuliaGPU/repos\",\"events_url\":\"https://api.github.com/users/JuliaGPU/events{/privacy}\",\"received_events_url\":\"https://api.github.com/users/JuliaGPU/received_events\",\"type\":\"Organization\",\"site_admin\":false},\"private\":false,\"html_url\":\"https://github.com/JuliaGPU/CUDAdrv.jl\",\"description\":\"A Julia wrapper for the CUDA driver API.\",\"fork\":false,\"url\":\"https://github.com/JuliaGPU/CUDAdrv.jl\",\"forks_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/forks\",\"keys_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/keys{/key_id}\",\"collaborators_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/collaborators{/collaborator}\",\"teams_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/teams\",\"hooks_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/hooks\",\"issue_events_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/issues/events{/number}\",\"events_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/events\",\"assignees_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/assignees{/user}\",\"branches_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/branches{/branch}\",\"tags_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/tags\",\"blobs_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/git/blobs{/sha}\",\"git_tags_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/git/tags{/sha}\",\"git_refs_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/git/refs{/sha}\",\"trees_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/git/trees{/sha}\",\"statuses_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/statuses/{sha}\",\"languages_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/languages\",\"stargazers_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/stargazers\",\"contributors_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/contributors\",\"subscribers_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/subscribers\",\"subscription_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/subscription\",\"commits_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/commits{/sha}\",\"git_commits_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/git/commits{/sha}\",\"comments_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/comments{/number}\",\"issue_comment_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/issues/comments{/number}\",\"contents_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/contents/{+path}\",\"compare_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/compare/{base}...{head}\",\"merges_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/merges\",\"archive_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/{archive_format}{/ref}\",\"downloads_
[DEBUG - HTTP./opt/pkgs/v0.6/HTTP/src/parser.jl:107  ]: String(bytes[i:j]) = url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/downloads\",\"issues_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/issues{/number}\",\"pulls_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/pulls{/number}\",\"milestones_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/milestones{/number}\",\"notifications_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/notifications{?since,all,participating}\",\"labels_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/labels{/name}\",\"releases_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/releases{/id}\",\"deployments_url\":\"https://api.github.com/repos/JuliaGPU/CUDAdrv.jl/deployments\",\"created_at\":1456220762,\"updated_at\":\"2017-12-30T13:37:28Z\",\"pushed_at\":1515691293,\"git_url\":\"git://github.com/JuliaGPU/CUDAdrv.jl.git\",\"ssh_url\":\"[email protected]:JuliaGPU/CUDAdrv.jl.git\",\"clone_url\":\"https://github.com/JuliaGPU/CUDAdrv.jl.git\",\"svn_url\":\"https://github.com/JuliaGPU/CUDAdrv.jl\",\"homepage\":\"\",\"size\":874,\"stargazers_count\":21,\"watchers_count\":21,\"language\":\"Julia\",\"has_issues\":true,\"has_projects\":false,\"has_downloads\":true,\"has_wiki\":false,\"has_pages\":true,\"forks_count\":9,\"mirror_url\":null,\"archived\":false,\"open_issues_count\":4,\"license\":{\"key\":\"mit\",\"name\":\"MIT License\",\"spdx_id\":\"MIT\",\"url\":\"https://api.github.com/licenses/mit\"},\"forks\":9,\"open_issues\":4,\"watchers\":21,\"default_branch\":\"master\",\"stargazers\":21,\"master_branch\":\"master\",\"organization\":\"JuliaGPU\"},\"pusher\":{\"name\":\"maleadt\",\"email\":\"[email protected]\"},\"organization\":{\"login\":\"JuliaGPU\",\"id\":7346142,\"url\":\"https://api.github.com/orgs/JuliaGPU\",\"repos_url\":\"https://api.github.com/orgs/JuliaGPU/repos\",\"events_url\":\"https://api.github.com/orgs/JuliaGPU/events\",\"hooks_url\":\"https://api.github.com/orgs/JuliaGPU/hooks\",\"issues_url\":\"https://api.github.com/orgs/JuliaGPU/issues\",\"members_url\":\"https://api.github.com/orgs/JuliaGPU/members{/member}\",\"public_members_url\":\"https://api.github.com/orgs/JuliaGPU/public_members{/member}\",\"avatar_url\":\"https://avatars2.githubusercontent.com/u/7346142?v=4\",\"description\":\"GPU Computing for Julia\"},\"sender\":{\"login\":\"maleadt\",\"id\":383068,\"avatar_url\":\"https://avatars3.githubusercontent.com/u/383068?v=4\",\"gravatar_id\":\"\",\"url\":\"https://api.github.com/users/maleadt\",\"html_url\":\"https://github.com/maleadt\",\"followers_url\":\"https://api.github.com/users/maleadt/followers\",\"following_url\":\"https://api.github.com/users/maleadt/following{/other_user}\",\"gists_url\":\"https://api.github.com/users/maleadt/gists{/gist_id}\",\"starred_url\":\"https://api.github.com/users/maleadt/starred{/owner}{/repo}\",\"subscriptions_url\":\"https://api.github.com/users/maleadt/subscriptions\",\"organizations_url\":\"https://api.github.com/users/maleadt/orgs\",\"repos_url\":\"https://api.github.com/users/maleadt/repos\",\"events_url\":\"https://api.github.com/users/maleadt/events{/privacy}\",\"received_events_url\":\"https://api.github.com/users/maleadt/received_events\",\"type\":\"User\",\"site_admin\":false},\"installation\":{\"id\":45739}}
[DEBUG - HTTP./opt/pkgs/v0.6/HTTP/src/parser.jl:107  ]: String(r.body) = url\":\"

[samoconnor]

Hi @Keno,

I found a few similar parser bugs related to fragmentation: #125, #126 and wrote some test cases for fragmented parsing: #124.

Since then I've been in a loop of: fix-simple-bugs/more-severe-tests/fix-by-refactoring/more-severe-tests...
The results of this are here: #135

Please feel free to give my branch a try, I'd be happy to help debugging any problems you encounter.

Some examples of the tests:

• S3: PUT and GET with hundreds of async tasks: https://github.com/samoconnor/HTTP.jl/blob/simplify_parser_branch/test/async.jl#L83
• Chunked GET from httpbin.org with hundreds of async tasks and randomised use of HTTP.open, HTTP.request and HTTP.request(response_stream=) APIs: https://github.com/samoconnor/HTTP.jl/blob/simplify_parser_branch/test/async.jl#L243
• Audio streaming over WebSockets: Integrate w/ HTTP.jl WebSockets.jl#84 (comment)
• loopback tests for pipelining and RFC edge cases (no network comms): https://github.com/samoconnor/HTTP.jl/blob/simplify_parser_branch/test/loopback.jl

[quinnj]

Yeah, I was going to comment that @samoconnor has been doing a ton of stress testing on his new branch/PR. What's the overall status of that branch @samoconnor? Would it be more worth my time to dive in reviewing that and helping it get merged? Or do you feel like there's still quite a bit of work to do on it?

[samoconnor]

Hi @quinnj, I've now got working examples of server side stuff. I tweaked HTTP.Stream, HTTP.Transaction and HTTP.Connection a little to be more symmetrical to get this working nicely. I'm no longer using the parser to parse the body unless chunked mode is used (it's more efficient to just read/write the TCP stream when not in chunked mode).
I've also done a post-server-implementation review pass of the parser and removed a bunch of code that is no-longer used now that HTTP.Message can figure out its own bodylength and HTTP.Stream takes care of read/writing the body. It is starting to feel like the interfaces between the modules are simple and easy to explain.

The server code is simpler now that HTTP.Stream, HTTP.Transaction and HTTP.Connection do all the work. https://github.com/samoconnor/HTTP.jl/blob/simplify_parser_branch/src/server.jl#L387
Having this code shared between client and server feels right and having it working for both feels like it is close to being finished.

In short, yes, I think it would be great if you could spend some time diving into the code and/or testing it. I don't think I'll make more structural changes. My next step is to start using AWSCore stuff on top of my branch in a real-world project. So, I hope it's all bug fixing from here.

The only outstanding tasks that come to mind are:

• Integrating the handler stuff with the new HTTP.listen() API.
• Checking compatibility with old API / deprecation warnings etc.

See example of server side stuff here: JuliaWeb/WebSockets.jl#84 (comment)

[EricForgy]

I would love to help out with testing. I am using HttpServer pretty heavily at the moment, but I need WebSockets 😅 When JuliaWeb/WebSockets.jl#84 (comment) is settled, then I'll be excited to try it out 😊

[Keno]

FWIW, I haven't seen this one after the merge, so I'll consider this fixed.

[samoconnor]

Hi @Keno sounds good.
The tests now feed fragmented test messages to the parser with many different fragment sizes, so I think we're well covered in this respect.