From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS12876 163.172.0.0/16 X-Spam-Status: No, score=-2.5 required=3.0 tests=AWL,BAYES_00, RCVD_IN_MSPIKE_BL,RCVD_IN_MSPIKE_ZBI,RCVD_IN_XBL,SPF_FAIL,SPF_HELO_FAIL shortcircuit=no autolearn=no autolearn_force=no version=3.4.0 Received: from 80x24.org (tor-exit-readme.memcpy.io [163.172.67.180]) by dcvr.yhbt.net (Postfix) with ESMTP id 0C3F4207C5 for ; Wed, 19 Apr 2017 22:30:49 +0000 (UTC) From: Eric Wong To: kcar-public@bogomips.org Subject: [PATCH 7/7] flesh out filter_body for request parsing Date: Wed, 19 Apr 2017 22:30:25 +0000 Message-Id: <20170419223025.8093-8-e@80x24.org> In-Reply-To: <20170419223025.8093-1-e@80x24.org> References: <20170419223025.8093-1-e@80x24.org> List-Id: Relying on Kcar::Parser#body_bytes_left= is still more efficient, but allowing filter_body to work for identity request bodies should simplify server implementations at the moment. When parsing responses, identity request bodies will still fail on filter_body, at least for now. It's always been that way, and it might be easier-to-write backwards compatible code in case people want to stick to an older version of kcar. --- ext/kcar/kcar.rl | 41 ++++- test/test_request_parser.rb | 361 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 398 insertions(+), 4 deletions(-) diff --git a/ext/kcar/kcar.rl b/ext/kcar/kcar.rl index 42eed9d..35997c1 100644 --- a/ext/kcar/kcar.rl +++ b/ext/kcar/kcar.rl @@ -201,6 +201,13 @@ static void finalize_header(struct http_parser *hp, VALUE hdr) if (hp->has_trailer && !hp->chunked) rb_raise(eParserError, "trailer but not chunked"); if (hp->is_request) { + if (hp->chunked) { + if (hp->len.chunk >= 0) + rb_raise(eParserError, "Content-Length set with chunked encoding"); + else + hp->len.chunk = 0; + } + if (!hp->has_query) rb_hash_aset(hdr, g_QUERY_STRING, rb_str_new(NULL, 0)); if (hp->has_header) { @@ -838,6 +845,8 @@ static VALUE body_bytes_left_set(VALUE self, VALUE bytes) if (hp->chunked) rb_raise(rb_eRuntimeError, "body_bytes_left= is not for chunked bodies"); hp->len.content = NUM2OFFT(bytes); + if (hp->len.content == 0) + hp->body_eof_seen = 1; return bytes; } @@ -980,8 +989,12 @@ static VALUE keepalive(VALUE self) if (hp->persistent) { if (hp->has_header && hp->has_body) { - if (hp->chunked || (hp->len.content >= 0)) - return Qtrue; + if (hp->chunked || (hp->len.content >= 0)) { + if (!hp->is_request) + return Qtrue; + else + return hp->body_eof_seen ? Qtrue : Qfalse; + } /* unknown Content-Length and not chunked, we must assume close */ return Qfalse; @@ -1019,12 +1032,34 @@ static VALUE filter_body(VALUE self, VALUE dst, VALUE src) StringValue(dst); rb_str_modify(dst); - rb_str_resize(dst, slen); /* we can never copy more than slen bytes */ OBJ_TAINT(dst); /* keep weirdo $SAFE users happy */ + /* + * for now, only support filter_body for identity requests, + * not responses; it's rather inefficient to blindly memcpy + * giant request bodies; on the other hand, it simplifies + * server-side code. + */ + if (hp->is_request && !hp->chunked) { + /* no need to enter the Ragel machine for unchunked transfers */ + assert(hp->len.content >= 0 && "negative Content-Length"); + if (hp->len.content > 0) { + long nr = MIN(slen, hp->len.content); + + rb_str_resize(dst, nr); + memcpy(RSTRING_PTR(dst), sptr, nr); + hp->len.content -= nr; + if (hp->len.content == 0) + hp->body_eof_seen = 1; + advance_str(src, nr); + } + return dst; + } + if (!hp->chunked) rb_raise(rb_eRuntimeError, "filter_body is only for chunked bodies"); + rb_str_resize(dst, slen); /* we can never copy more than slen bytes */ if (!chunked_eof(hp)) { hp->s.dest_offset = 0; http_parser_execute(hp, dst, sptr, slen); diff --git a/test/test_request_parser.rb b/test/test_request_parser.rb index 2bbcffd..18a6d71 100644 --- a/test/test_request_parser.rb +++ b/test/test_request_parser.rb @@ -730,7 +730,7 @@ class TestRequestParser < Test::Unit::TestCase req = @hp.request(@env, str) assert_equal 'Content-MD5,Content-SHA1', req['HTTP_TRAILER'] assert_equal "1\r\na\r\n2\r\n..\r\n0\r\n", str - assert @hp.keepalive? + assert_not_predicate @hp, :keepalive? end def test_http_09 @@ -857,4 +857,363 @@ class TestRequestParser < Test::Unit::TestCase assert_equal expect, env2 assert_equal '', buf end + + def test_identity_byte_headers + str = "PUT / HTTP/1.1\r\n" + str << "Content-Length: 123\r\n" + str << "\r" + buf = '' + str.each_byte { |byte| + buf << byte.chr + assert_nil @hp.request(@env, str) + } + buf << "\n" + req = @hp.request(@env, buf) + assert_equal '123', req['CONTENT_LENGTH'] + assert_predicate buf, :empty? + assert ! @hp.keepalive? + assert_equal 123, @hp.body_bytes_left + dst = "" + buf = '.' * 123 + @hp.filter_body(dst, buf) + assert_equal '.' * 123, dst + assert_equal "", buf + assert_predicate @hp, :keepalive? + end + + def test_identity_step_headers + str = "PUT / HTTP/1.1\r\n" + assert_nil @hp.request(@env, str) + str << "Content-Length: 123\r\n" + assert_nil @hp.request(@env, str) + str << "\r\n" + req = @hp.request(@env, str) + assert_equal '123', req['CONTENT_LENGTH'] + assert_equal 0, str.size + assert_not_predicate @hp, :keepalive? + dst = "" + buf = '.' * 123 + @hp.filter_body(dst, buf) + assert_equal '.' * 123, dst + assert_equal "", buf + assert_predicate @hp, :keepalive? + end + + def test_identity_oneshot_header + str = "PUT / HTTP/1.1\r\nContent-Length: 123\r\n\r\n" + req = @hp.request(@env, str) + assert_equal '123', req['CONTENT_LENGTH'] + assert_equal 0, str.size + assert_not_predicate @hp, :keepalive? + dst = "" + buf = '.' * 123 + @hp.filter_body(dst, buf) + assert_equal '.' * 123, dst + assert_equal "", buf + assert_predicate @hp, :keepalive? + end + + def test_identity_oneshot_header_with_body + body = ('a' * 123).freeze + str = "PUT / HTTP/1.1\r\n" \ + "Content-Length: #{body.length}\r\n" \ + "\r\n#{body}" + req = @hp.request(@env, str) + assert_equal '123', req['CONTENT_LENGTH'] + assert_equal 123, str.size + assert_equal body, str + tmp = '' + assert_same tmp, @hp.filter_body(tmp, str) + assert_equal 0, str.size + assert_equal tmp, body + assert_equal body, @hp.filter_body(tmp, str) + assert_predicate @hp, :keepalive? + end + + def test_identity_oneshot_header_with_body_partial + str = "PUT / HTTP/1.1\r\nContent-Length: 123\r\n\r\na" + req = @hp.request(@env, str) + assert_equal 1, str.size + assert_equal 'a', str + tmp = '' + assert_same tmp, @hp.filter_body(tmp, str) + assert_equal "", str + assert_equal "a", tmp + str << ' ' * 122 + rv = @hp.filter_body(tmp, str) + assert_equal 122, tmp.size + assert_same tmp, rv + assert_equal "", str + assert_same tmp, @hp.filter_body(tmp, str) + assert_predicate @hp, :keepalive? + assert_equal '123', req['CONTENT_LENGTH'] + end + + def test_identity_oneshot_header_with_body_slop + str = "PUT / HTTP/1.1\r\nContent-Length: 1\r\n\r\naG" + req = @hp.request(@env, str) + assert_equal 2, str.size + assert_equal 'aG', str + tmp = '' + assert_same tmp, @hp.filter_body(tmp, str) + assert_equal "G", str + assert_equal "a", @hp.filter_body(tmp, str) + assert_equal 1, tmp.size + assert_equal "a", tmp + assert_predicate @hp, :keepalive? + assert_equal '1', req['CONTENT_LENGTH'] + end + + def test_chunked + str = "PUT / HTTP/1.1\r\ntransfer-Encoding: chunked\r\n\r\n" + req = @hp.request(@env, str) + assert_equal 0, str.size + tmp = "" + assert_predicate @hp, :chunked? + assert_nil @hp.filter_body(tmp, str << "6") + assert_equal 0, tmp.size + assert_nil @hp.filter_body(tmp, str << "\r\n") + assert_equal 0, str.size + assert_equal 0, tmp.size + tmp = "" + assert_nil @hp.filter_body(tmp, str << "..") + assert_equal "..", tmp + assert_nil @hp.filter_body(tmp, str << "abcd\r\n0\r\n") + assert_equal "abcd", tmp + assert_same tmp, @hp.filter_body(tmp, str << "PUT") + assert_equal "PUT", str + assert_not_predicate @hp, :keepalive? + str << "TY: FOO\r\n\r\n" + req = @hp.request(@env, str) + assert_equal "FOO", req["HTTP_PUTTY"] + assert_predicate @hp, :keepalive? + end + + def test_chunked_empty + str = "PUT / HTTP/1.1\r\ntransfer-Encoding: chunked\r\n\r\n" + req = @hp.request(@env, str) + assert_equal 0, str.size + tmp = "" + assert_same tmp, @hp.filter_body(tmp, str << "0\r\n\r\n") + assert_predicate @hp, :body_eof? + assert_equal "", tmp + assert_same req, @hp.request(@env, str) + assert_equal "", str + end + + def test_two_chunks + str = "PUT / HTTP/1.1\r\ntransfer-Encoding: chunked\r\n\r\n" + req = @hp.request(@env, str) + assert_same req, @env + assert_equal 0, str.size + tmp = "" + assert_nil @hp.filter_body(tmp, str << "6") + assert_equal 0, tmp.size + assert_nil @hp.filter_body(tmp, str << "\r\n") + assert_equal "", str + assert_equal 0, tmp.size + tmp = "" + assert_nil @hp.filter_body(tmp, str << "..") + assert_equal 2, tmp.size + assert_equal "..", tmp + assert_nil @hp.filter_body(tmp, str << "abcd\r\n1") + assert_equal "abcd", tmp + assert_nil @hp.filter_body(tmp, str << "\r") + assert_equal "", tmp + assert_nil @hp.filter_body(tmp, str << "\n") + assert_equal "", tmp + assert_nil @hp.filter_body(tmp, str << "z") + assert_equal "z", tmp + assert_nil @hp.filter_body(tmp, str << "\r\n") + assert_nil @hp.filter_body(tmp, str << "0") + assert_nil @hp.filter_body(tmp, str << "\r") + rv = @hp.filter_body(tmp, str << "\nGET") + assert_equal "GET", str + assert_same tmp, rv + assert_equal '', tmp + assert_not_predicate @hp, :keepalive? + end + + def test_big_chunk + str = "PUT / HTTP/1.1\r\ntransfer-Encoding: chunked\r\n\r\n" \ + "4000\r\nabcd" + req = @hp.request(@env, str) + tmp = '' + assert_nil @hp.filter_body(tmp, str) + assert_equal '', str + str << ' ' * 16300 + assert_nil @hp.filter_body(tmp, str) + assert_equal '', str + str << ' ' * 80 + assert_nil @hp.filter_body(tmp, str) + assert_equal '', str + assert_not_predicate @hp, :body_eof? + assert_same tmp, @hp.filter_body(tmp, str << "\r\n0\r\n") + assert_equal "", tmp + assert_predicate @hp, :body_eof? + str << "\r\n" + assert_same req, @hp.request(@env, str) + assert_equal "", str + assert_predicate @hp, :body_eof? + assert_predicate @hp, :keepalive? + end + + def test_two_chunks_oneshot + str = "PUT / HTTP/1.1\r\ntransfer-Encoding: chunked\r\n\r\n" \ + "1\r\na\r\n2\r\n..\r\n0\r\n" + req = @hp.request(@env, str) + assert_same req, @env + tmp = '' + assert_nil @hp.filter_body(tmp, str) + assert_equal 'a..', tmp + assert_same tmp, @hp.filter_body(tmp, str) + assert_not_predicate @hp, :keepalive? + end + + def test_chunks_bytewise + chunked = "10\r\nabcdefghijklmnop\r\n11\r\n0123456789abcdefg\r\n0\r\n" + str = "PUT / HTTP/1.1\r\ntransfer-Encoding: chunked\r\n\r\n" + buf = str.dup + req = @hp.request(@env, buf) + assert_same req, @env + assert_equal "", buf + tmp = '' + body = '' + str = chunked[0..-2] + str.each_byte { |byte| + assert_nil @hp.filter_body(tmp, buf << byte.chr) + body << tmp + } + assert_equal 'abcdefghijklmnop0123456789abcdefg', body + assert_same tmp, @hp.filter_body(tmp, buf << "\n") + assert_not_predicate @hp, :keepalive? + end + + def test_trailers + str = "PUT / HTTP/1.1\r\n" \ + "Trailer: Content-MD5\r\n" \ + "transfer-Encoding: chunked\r\n\r\n" \ + "1\r\na\r\n2\r\n..\r\n0\r\n" + req = @hp.request(@env, str) + assert_same req, @env + assert_equal 'Content-MD5', req['HTTP_TRAILER'] + assert_nil req['HTTP_CONTENT_MD5'] + tmp = '' + assert_nil @hp.filter_body(tmp, str) + assert_equal 'a..', tmp + md5_b64 = [ Digest::MD5.digest(tmp) ].pack('m').strip.freeze + assert_same tmp, @hp.filter_body(tmp, str) + assert_equal '', str + md5_hdr = "Content-MD5: #{md5_b64}\r\n".freeze + str << md5_hdr + assert_nil @hp.request(@env, str) + assert_equal md5_b64, req['HTTP_CONTENT_MD5'] + assert_equal "CONTENT_MD5: #{md5_b64}\r\n", str + str << "\r" + assert_nil @hp.request(@env, str) + str << "\nGET / " + req = @hp.request(@env, str) + assert_equal "GET / ", str + assert_predicate @hp, :keepalive? + end + + def test_trailers_slowly + str = "PUT / HTTP/1.1\r\n" \ + "Trailer: Content-MD5\r\n" \ + "transfer-Encoding: chunked\r\n\r\n" \ + "1\r\na\r\n2\r\n..\r\n0\r\n" + req = @hp.request(@env, str) + assert_equal 'Content-MD5', req['HTTP_TRAILER'] + assert_nil req['HTTP_CONTENT_MD5'] + tmp = '' + assert_nil @hp.filter_body(tmp, str) + assert_equal 'a..', tmp + md5_b64 = [ Digest::MD5.digest(tmp) ].pack('m').strip.freeze + assert_same tmp, @hp.filter_body(tmp, str) + assert_equal '', str + assert_nil @hp.request(req, str) + md5_hdr = "Content-MD5: #{md5_b64}\r\n".freeze + md5_hdr.each_byte { |byte| + str << byte.chr + assert_nil @hp.request(req, str) + } + assert_equal md5_b64, req['HTTP_CONTENT_MD5'] + assert_equal "CONTENT_MD5: #{md5_b64}\r\n", str + str << "\r" + assert_nil @hp.request(@env, str) + str << "\n" + assert_same req, @hp.request(@env, str) + end + + def test_max_chunk + assert_operator Kcar::Parser::CHUNK_MAX, :>=, 0xffff + str = "PUT / HTTP/1.1\r\n" \ + "transfer-Encoding: chunked\r\n\r\n" \ + "#{Kcar::Parser::CHUNK_MAX.to_s(16)}\r\na\r\n2\r\n..\r\n0\r\n" + req = @hp.request(@env, str) + assert_same req, @env + assert_nil @hp.body_bytes_left + assert_nil @hp.filter_body('', str) + assert_not_predicate @hp, :keepalive? + end + + def test_max_body + n = Kcar::Parser::LENGTH_MAX + buf = "PUT / HTTP/1.1\r\nContent-Length: #{n}\r\n\r\n" + req = @hp.request(@env, buf) + assert_equal n, req['CONTENT_LENGTH'].to_i + # connection can only be persistent when body is drained: + assert_not_predicate @hp, :keepalive? + @hp.body_bytes_left = 1 + assert_not_predicate @hp, :keepalive? + @hp.body_bytes_left = 0 + assert_predicate @hp, :keepalive? + end + + def test_overflow_chunk + n = Kcar::Parser::CHUNK_MAX + 1 + str = "PUT / HTTP/1.1\r\n" \ + "transfer-Encoding: chunked\r\n\r\n" \ + "#{n.to_s(16)}\r\na\r\n2\r\n..\r\n0\r\n" + req = @hp.request(@env, str) + assert_nil @hp.body_bytes_left + assert_raise(Kcar::ParserError) { @hp.filter_body('', str) } + end + + def test_overflow_content_length + n = Kcar::Parser::LENGTH_MAX + 1 + buf ="PUT / HTTP/1.1\r\nContent-Length: #{n}\r\n\r\n" + assert_raise(Kcar::ParserError) do + @hp.request(@env, buf) + end + end + + def test_bad_chunk + buf = "PUT / HTTP/1.1\r\n" \ + "transfer-Encoding: chunked\r\n\r\n" \ + "#zzz\r\na\r\n2\r\n..\r\n0\r\n" + @hp.request(@env, buf) + assert_nil @hp.body_bytes_left + assert_raise(Kcar::ParserError) { @hp.filter_body("", buf) } + end + + def test_bad_content_length + buf = "PUT / HTTP/1.1\r\nContent-Length: 7ff\r\n\r\n" + assert_raise(Kcar::ParserError) { @hp.request(@env, buf) } + end + + def test_bad_trailers + str = "PUT / HTTP/1.1\r\n" \ + "Trailer: Transfer-Encoding\r\n" \ + "transfer-Encoding: chunked\r\n\r\n" \ + "1\r\na\r\n2\r\n..\r\n0\r\n" + req = @hp.request(@env, str) + assert_equal 'Transfer-Encoding', req['HTTP_TRAILER'] + tmp = '' + assert_nil @hp.filter_body(tmp, str) + assert_equal 'a..', tmp + assert_equal '', str + str << "Transfer-Encoding: identity\r\n\r\n" + assert_raise(Kcar::ParserError) { @hp.request(@env, str) } + end end -- EW