From 94312009afb7f394806df013cd486430d92f9bd3 Mon Sep 17 00:00:00 2001 From: Sam O'Connor Date: Wed, 22 Nov 2017 18:58:29 +1100 Subject: [PATCH] Fix #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. --- src/parser.jl | 73 ++++++++++++++++++++++++++++------------------- src/precompile.jl | 10 ++++--- src/server.jl | 4 +-- test/parser.jl | 14 ++++++--- 4 files changed, 62 insertions(+), 39 deletions(-) diff --git a/src/parser.jl b/src/parser.jl index 438a19afb..179865e6d 100644 --- a/src/parser.jl +++ b/src/parser.jl @@ -31,9 +31,10 @@ mutable struct Parser content_length::UInt64 fieldbuffer::Vector{UInt8} valuebuffer::Vector{UInt8} + previous_field::Ref{String} end -Parser() = Parser(s_start_req_or_res, 0x00, 0, 0, 0, 0, UInt8[], UInt8[]) +Parser() = Parser(s_start_req_or_res, 0x00, 0, 0, 0, 0, UInt8[], UInt8[], Ref{String}()) function reset!(p::Parser) p.state = start_state @@ -44,6 +45,7 @@ function reset!(p::Parser) p.content_length = 0x0000000000000000 empty!(p.fieldbuffer) empty!(p.valuebuffer) + p.previous_field = Ref{String}() return end @@ -55,15 +57,23 @@ macro nread(n) end onmessagebegin(r) = @debug(PARSING_DEBUG, "onmessagebegin") + +function onurlbytes(p::Parser, bytes, i, j) + @debug(PARSING_DEBUG, "onurlbytes") + append!(p.valuebuffer, view(bytes, i:j)) + return +end + # should we just make a copy of the byte vector for URI here? -function onurl(r, bytes, i, j) +function onurl(p::Parser, r) @debug(PARSING_DEBUG, "onurl") - @debug(PARSING_DEBUG, i - j + 1) - @debug(PARSING_DEBUG, "'$(String(bytes[i:j]))'") + @debug(PARSING_DEBUG, "'$(String(p.valuebuffer))'") @debug(PARSING_DEBUG, r.method) - uri = URIs.http_parser_parse_url(bytes, i, j - i + 1, r.method == CONNECT) + url = copy(p.valuebuffer) + uri = URIs.http_parser_parse_url(url, 1, length(url), r.method == CONNECT) @debug(PARSING_DEBUG, uri) setfield!(r, :uri, uri) + empty!(p.valuebuffer) return end onstatus(r) = @debug(PARSING_DEBUG, "onstatus") @@ -77,7 +87,7 @@ function onheadervalue(p::Parser, bytes, i, j) append!(p.valuebuffer, view(bytes, i:j)) return end -function onheadervalue(p, r, bytes, i, j, issetcookie, host, KEY, canonicalizeheaders) +function onheadervalue(p, r, bytes, i, j, issetcookie, host, canonicalizeheaders) @debug(PARSING_DEBUG, "onheadervalue2") append!(p.valuebuffer, view(bytes, i:j)) if canonicalizeheaders @@ -88,13 +98,13 @@ function onheadervalue(p, r, bytes, i, j, issetcookie, host, KEY, canonicalizehe val = unsafe_string(pointer(p.valuebuffer), length(p.valuebuffer)) if key == "" # the header value was parsed in two parts, - # KEY[] holds the most recently parsed header field, + # p.previous_field[] holds the most recently parsed header field, # and we already stored the first part of the header value in r.headers # get the first part and concatenate it with the second part we have now - key = KEY[] + key = p.previous_field[] setindex!(r.headers, string(r.headers[key], val), key) else - KEY[] = key + p.previous_field[] = key val2 = get!(r.headers, key, val) val2 !== val && setindex!(r.headers, string(val2, ", ", val), key) end @@ -151,7 +161,9 @@ function parse(T::Type{<:Union{Request, Response}}, str; err, headerscomplete, messagecomplete, upgrade = parse!(r, DEFAULT_PARSER, Vector{UInt8}(str); lenient=lenient, maxuri=maxuri, maxheader=maxheader, maxbody=maxbody, maintask=maintask, canonicalizeheaders=canonicalizeheaders) err != HPE_OK && throw(ParsingError("error parsing $T: $(ParsingErrorCodeMap[err])")) - extra[] = upgrade + if isassigned(upgrade) + extra[] = upgrade[] + end return r end @@ -165,7 +177,7 @@ function parse!(r::Union{Request, Response}, parser, bytes, len=length(bytes); lenient::Bool=true, host::String="", method::Method=GET, maxuri::Int64=DEFAULT_MAX_URI, maxheader::Int64=DEFAULT_MAX_HEADER, maxbody::Int64=DEFAULT_MAX_BODY, maintask::Task=current_task(), - canonicalizeheaders::Bool=true)::Tuple{ParsingErrorCode, Bool, Bool, String} + canonicalizeheaders::Bool=true)::Tuple{ParsingErrorCode, Bool, Bool, Ref{String}} return parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, maxbody, maintask, canonicalizeheaders) end function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, maxbody, maintask, canonicalizeheaders) @@ -174,7 +186,6 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, status_mark = url_mark = header_field_mark = header_field_end_mark = header_value_mark = body_mark = 0 errno = HPE_OK upgrade = issetcookie = headersdone = false - KEY = Ref{String}() @debug(PARSING_DEBUG, len) @debug(PARSING_DEBUG, ParsingStateCode(p_state)) if len == 0 @@ -182,11 +193,11 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, parser.state = p_state onmessagecomplete(r) @debug(PARSING_DEBUG, "this 6") - return HPE_OK, true, true, "" + return HPE_OK, true, true, Ref{String}() elseif @anyeq(p_state, s_dead, s_start_req_or_res, s_start_res, s_start_req) - return HPE_OK, false, false, "" + return HPE_OK, false, false, Ref{String}() else - return HPE_INVALID_EOF_STATE, false, false, "" + return HPE_INVALID_EOF_STATE, false, false, Ref{String}() end end @@ -200,7 +211,8 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, end if @anyeq(p_state, s_req_path, s_req_schema, s_req_schema_slash, s_req_schema_slash_slash, s_req_server_start, s_req_server, s_req_server_with_at, - s_req_query_string_start, s_req_query_string, s_req_fragment) + s_req_query_string_start, s_req_query_string, s_req_fragment, + s_req_fragment_start) url_mark = 1 elseif p_state == s_res_status status_mark = 1 @@ -511,7 +523,8 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, p_state = s_req_http_start parser.state = p_state p - url_mark > maxuri && @err(HPE_URI_OVERFLOW) - onurl(r, bytes, url_mark, p-1) + onurlbytes(parser, bytes, url_mark, p-1) + onurl(parser, r) url_mark = 0 elseif ch in (CR, LF) r.major = Int16(0) @@ -519,7 +532,8 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, p_state = ifelse(ch == CR, s_req_line_almost_done, s_header_field_start) parser.state = p_state p - url_mark > maxuri && @err(HPE_URI_OVERFLOW) - onurl(r, bytes, url_mark, p-1) + onurlbytes(parser, bytes, url_mark, p-1) + onurl(parser, r) url_mark = 0 else p_state = URIs.parseurlchar(p_state, ch, strict) @@ -812,7 +826,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, parser.header_state = h parser.state = p_state @debug(PARSING_DEBUG, "onheadervalue 1") - onheadervalue(parser, r, bytes, header_value_mark, p - 1, issetcookie, host, KEY, canonicalizeheaders) + onheadervalue(parser, r, bytes, header_value_mark, p - 1, issetcookie, host, canonicalizeheaders) header_value_mark = 0 break elseif ch == LF @@ -821,7 +835,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, parser.header_state = h parser.state = p_state @debug(PARSING_DEBUG, "onheadervalue 2") - onheadervalue(parser, r, bytes, header_value_mark, p - 1, issetcookie, host, KEY, canonicalizeheaders) + onheadervalue(parser, r, bytes, header_value_mark, p - 1, issetcookie, host, canonicalizeheaders) header_value_mark = 0 @goto reexecute elseif !lenient && !isheaderchar(ch) @@ -1022,7 +1036,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, p_state = s_header_field_start parser.state = p_state @debug(PARSING_DEBUG, "onheadervalue 3") - onheadervalue(parser, r, bytes, header_value_mark, p - 1, issetcookie, host, KEY, canonicalizeheaders) + onheadervalue(parser, r, bytes, header_value_mark, p - 1, issetcookie, host, canonicalizeheaders) header_value_mark = 0 @goto reexecute end @@ -1083,7 +1097,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, parser.state = p_state onmessagecomplete(r) @debug(PARSING_DEBUG, "this 1") - return errno, true, true, String(bytes[p+1:end]) + return errno, true, true, Ref{String}(String(bytes[p+1:end])) end if parser.flags & F_SKIPBODY > 0 @@ -1091,7 +1105,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, parser.state = p_state onmessagecomplete(r) @debug(PARSING_DEBUG, "this 2") - return errno, true, true, "" + return errno, true, true, Ref{String}() elseif parser.flags & F_CHUNKED > 0 #= chunked encoding - ignore Content-Length header =# p_state = s_chunk_size_start @@ -1102,7 +1116,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, parser.state = p_state onmessagecomplete(r) @debug(PARSING_DEBUG, "this 3") - return errno, true, true, "" + return errno, true, true, Ref{String}() elseif parser.content_length != ULLONG_MAX #= Content-Length header given and non-zero =# p_state = s_body_identity @@ -1114,7 +1128,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, parser.state = p_state onmessagecomplete(r) @debug(PARSING_DEBUG, "this 4") - return errno, true, true, String(bytes[p+1:end]) + return errno, true, true, Ref{String}(String(bytes[p+1:end])) else #= Read body until EOF =# p_state = s_body_identity_eof @@ -1171,7 +1185,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, if upgrade #= Exit, the rest of the message is in a different protocol. =# parser.state = p_state - return errno, true, true, String(bytes[p+1:end]) + return errno, true, true, Ref{String}(String(bytes[p+1:end])) end elseif p_state == s_chunk_size_start @@ -1299,7 +1313,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, @debug(PARSING_DEBUG, p) header_value_mark > 0 && onheadervalue(parser, bytes, header_value_mark, min(len, p)) url_mark > 0 && (min(len, p) - url_mark > maxuri) && @err(HPE_URI_OVERFLOW) - url_mark > 0 && onurl(r, bytes, url_mark, min(len, p)) + url_mark > 0 && onurlbytes(parser, bytes, url_mark, min(len, p)) @debug(PARSING_DEBUG, "this onbody 3") body_mark > 0 && onbody(r, maintask, bytes, body_mark, min(len, p - 1)) status_mark > 0 && onstatus(r) @@ -1310,7 +1324,8 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, b = p_state == start_state || p_state == s_dead he = b | (headersdone || p_state >= s_headers_done) m = b | (p_state >= s_message_done) - return errno, he, m, String(bytes[p:end]) + ug = p >= len ? Ref{String}() : Ref{String}(String(bytes[p:end])) + return errno, he, m, ug @label error if errno == HPE_OK @@ -1321,7 +1336,7 @@ function parse!(r, parser, bytes, len, lenient, host, method, maxuri, maxheader, parser.header_state = 0x00 @debug(PARSING_DEBUG, "exiting due to error...") @debug(PARSING_DEBUG, errno) - return errno, false, false, "" + return errno, false, false, Ref{String}() end #= Does the parser need to see an EOF to find the end of the message? =# diff --git a/src/precompile.jl b/src/precompile.jl index 165b2c3e9..f6f0e0291 100644 --- a/src/precompile.jl +++ b/src/precompile.jl @@ -5,9 +5,11 @@ function _precompile_() @assert precompile(HTTP.Cookies.pathmatch, (HTTP.Cookies.Cookie, String,)) @assert precompile(HTTP.onheaderfield, (HTTP.Parser, Array{UInt8, 1}, Int64, Int64,)) @assert precompile(HTTP.isjson, (Array{UInt8, 1}, UInt64, Int64,)) - @assert precompile(HTTP.onheadervalue, (HTTP.Parser, HTTP.Response, Array{UInt8, 1}, Int64, Int64, Bool, String, Base.RefValue{String}, Bool)) + @assert precompile(HTTP.onheadervalue, (HTTP.Parser, HTTP.Response, Array{UInt8, 1}, Int64, Int64, Bool, String, Bool)) @assert precompile(HTTP.isjson, (Array{UInt8, 1}, Int64, Int64,)) - @assert precompile(HTTP.onurl, (HTTP.Response, Array{UInt8, 1}, Int64, Int64,)) + @assert precompile(HTTP.onurlbytes, (HTTP.Parser, Array{UInt8, 1}, Int64, Int64,)) + @assert precompile(HTTP.onurl, (HTTP.Parser, HTTP.Response,)) + @assert precompile(HTTP.onurl, (HTTP.Parser, HTTP.Request,)) @assert precompile(HTTP.Response, (Int64, String,)) @assert precompile(HTTP.URIs.getindex, (Array{UInt8, 1}, HTTP.URIs.Offset,)) @assert precompile(HTTP.iscompressed, (Array{UInt8, 1},)) @@ -83,7 +85,7 @@ function _precompile_() @assert precompile(HTTP.sniff, (Array{UInt8, 1},)) @assert precompile(HTTP.mark, (HTTP.Multipart{Base.IOStream},)) @assert precompile(HTTP.seek, (HTTP.Form, Int64,)) - @assert precompile(HTTP.onheadervalue, (HTTP.Parser, HTTP.Request, Array{UInt8, 1}, Int64, Int64, Bool, String, Base.RefValue{String}, Bool)) + @assert precompile(HTTP.onheadervalue, (HTTP.Parser, HTTP.Request, Array{UInt8, 1}, Int64, Int64, Bool, String, Bool)) @assert precompile(HTTP.FIFOBuffers.write, (HTTP.FIFOBuffers.FIFOBuffer, String,)) @assert precompile(HTTP.URIs.escape, (String, String,)) @assert precompile(HTTP.dead!, (HTTP.Connection{MbedTLS.SSLContext},)) @@ -197,4 +199,4 @@ function _precompile_() @assert precompile(HTTP.startline, (Base.GenericIOBuffer{Array{UInt8, 1}}, HTTP.Request,)) end end -_precompile_() \ No newline at end of file +_precompile_() diff --git a/src/server.jl b/src/server.jl index b7a1d4bbb..5145539d6 100644 --- a/src/server.jl +++ b/src/server.jl @@ -150,7 +150,7 @@ function process!(server::Server{T, H}, parser, request, i, tcp, rl, starttime, response = HTTP.Response(417) error = true end - elseif length(upgrade) > 0 + elseif isassigned(upgrade) HTTP.@log "received upgrade request on connection i=$i" response = HTTP.Response(501, "upgrade requests are not currently supported") error = true @@ -338,4 +338,4 @@ serve(; host::IPAddr=IPv4(127,0,0,1), args...) = serve(host, port, handler, logger; cert=cert, key=key, verbose=verbose, args...) -end # module \ No newline at end of file +end # module diff --git a/test/parser.jl b/test/parser.jl index 1901ed247..ede1309bf 100644 --- a/test/parser.jl +++ b/test/parser.jl @@ -1364,7 +1364,10 @@ const responses = Message[ #@show [Char(x[i]) for i in 1:sz] err, hc, mc, ug = HTTP.parse!(r, p, x) err != HTTP.HPE_OK && throw(HTTP.ParsingError(HTTP.ParsingErrorCodeMap[err])) - upgrade[] = ug + if isassigned(ug) + upgrade[] = ug[] + break + end end else r = HTTP.parse(HTTP.Request, req.raw; extra=upgrade) @@ -1383,7 +1386,10 @@ const responses = Message[ @test HTTP.headers(r) == req.headers @test String(readavailable(HTTP.body(r))) == req.body @test HTTP.http_should_keep_alive(HTTP.DEFAULT_PARSER, r) == req.should_keep_alive - @test upgrade[] == req.upgrade + + @test t == "A" || + req.upgrade == "" && !isassigned(upgrade) || + upgrade[] == req.upgrade end reqstr = "GET http://www.techcrunch.com/ HTTP/1.1\r\n" * @@ -1639,7 +1645,7 @@ const responses = Message[ @test e == HTTP.HPE_OK @test !h @test !m - @test ex == "" + @test !isassigned(ex) buf = "header-key: header-value\r\n" for i = 1:10000 e, h, m, ex = HTTP.parse!(R, HTTP.DEFAULT_PARSER, Vector{UInt8}(r[2])) @@ -1721,7 +1727,7 @@ const responses = Message[ @test h @test m HTTP.reset!(p) - e, h, m, ex = HTTP.parse!(r, p, Vector{UInt8}(ex)) + e, h, m, ex = HTTP.parse!(r, p, Vector{UInt8}(ex[])) @test e == HTTP.HPE_OK @test h @test m