Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for parsing one charactar at a time, fix #125 and #126 #124

Closed
wants to merge 4 commits into from

Conversation

samoconnor
Copy link
Contributor

No description provided.

@samoconnor
Copy link
Contributor Author

Updated PR with fix for #125

@samoconnor samoconnor changed the title Add (failing) test for parsing one charactar at a time Add test for parsing one charactar at a time, fix #125 Nov 22, 2017
@codecov-io
Copy link

codecov-io commented Nov 22, 2017

Codecov Report

Merging #124 into master will decrease coverage by 36.56%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #124       +/-   ##
===========================================
- Coverage   75.76%   39.19%   -36.57%     
===========================================
  Files          15       15               
  Lines        2434     2396       -38     
===========================================
- Hits         1844      939      -905     
- Misses        590     1457      +867
Impacted Files Coverage Δ
src/parser.jl 76.03% <100%> (-0.66%) ⬇️
src/handlers.jl 0% <0%> (-93.19%) ⬇️
src/cookies.jl 0% <0%> (-92.6%) ⬇️
src/multipart.jl 0% <0%> (-91.94%) ⬇️
src/sniff.jl 0% <0%> (-83.34%) ⬇️
src/client.jl 1.05% <0%> (-78.95%) ⬇️
src/server.jl 0% <0%> (-72.96%) ⬇️
src/uri.jl 23.25% <0%> (-69.77%) ⬇️
src/fifobuffer.jl 30.15% <0%> (-65.08%) ⬇️
src/types.jl 12.14% <0%> (-64.29%) ⬇️
... and 4 more

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 779fe74...9431200. Read the comment docs.

@samoconnor
Copy link
Contributor Author

samoconnor commented Nov 22, 2017

updated with (failing) test for parsing requests 1 char at a time
See #126

Replace local `KEY` in `parse!()` with `parser.previous_field`.

Add onurlbytes(). Accumulates URL in `parser.valuebuffer`.

Use `Ref{String}` to return `extra`/`upgrade` so that caller can tell
the difference between upgrade with empty string and no upgrade.

Add `s_req_fragment_start` to list of states where the url_mark should
be reset to 1.
@samoconnor samoconnor changed the title Add test for parsing one charactar at a time, fix #125 Add test for parsing one charactar at a time, fix #125 and #126 Nov 22, 2017
@samoconnor
Copy link
Contributor Author

Updated with fix for #126

@quinnj
Copy link
Member

quinnj commented Nov 26, 2017

I'll need a few more days to review this; it touches several very low-level parts of the parser, so we want to be careful w/ performance and correctness.

@samoconnor
Copy link
Contributor Author

Thorough review sounds good.
Do you have a feel for whether #125 and #126 are inherited from the C code that you ported?, or if they are simply errors introduced while porting?

@samoconnor
Copy link
Contributor Author

#135

@samoconnor samoconnor closed this Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants