From f81aa02448b615c4d5fc4f6544c53289dae9d2ec Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 4 May 2011 16:41:36 -0700 Subject: return 414 for URI length violations There's an HTTP status code allocated for it in , so return that instead of 400. --- ext/unicorn_http/global_variables.h | 7 +++++ ext/unicorn_http/unicorn_http.rl | 35 ++++++++++++++++----- lib/unicorn/const.rb | 1 + lib/unicorn/http_server.rb | 2 ++ t/t0002-parser-error.sh | 62 ++++++++++++++++++++++++++++++++++++- 5 files changed, 98 insertions(+), 9 deletions(-) diff --git a/ext/unicorn_http/global_variables.h b/ext/unicorn_http/global_variables.h index cdbc42d..cda7f24 100644 --- a/ext/unicorn_http/global_variables.h +++ b/ext/unicorn_http/global_variables.h @@ -1,6 +1,7 @@ #ifndef global_variables_h #define global_variables_h static VALUE eHttpParserError; +static VALUE eRequestURITooLongError; static VALUE g_rack_url_scheme; static VALUE g_request_method; @@ -36,6 +37,7 @@ static VALUE g_http_11; "HTTP element " # N " is longer than the " # length " allowed length." NORETURN(static void parser_error(const char *)); +NORETURN(static void raise_414(const char *)); /** * Validates the max length of given input and throws an HttpParserError @@ -46,6 +48,11 @@ NORETURN(static void parser_error(const char *)); parser_error(MAX_##N##_LENGTH_ERR); \ } while (0) +#define VALIDATE_MAX_URI_LENGTH(len, N) do { \ + if (len > MAX_##N##_LENGTH) \ + raise_414(MAX_##N##_LENGTH_ERR); \ +} while (0) + /** Defines global strings in the init method. */ #define DEF_GLOBAL(N, val) do { \ g_##N = rb_obj_freeze(rb_str_new(val, sizeof(val) - 1)); \ diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl index 971cdc1..741f8ee 100644 --- a/ext/unicorn_http/unicorn_http.rl +++ b/ext/unicorn_http/unicorn_http.rl @@ -106,17 +106,32 @@ struct http_parser { } len; }; -static ID id_clear; +static ID id_clear, id_set_backtrace; static void finalize_header(struct http_parser *hp); +NORETURN(static void raise_with_empty_bt(VALUE)); + +static void raise_with_empty_bt(VALUE exc) +{ + VALUE bt = rb_ary_new(); + + rb_funcall(exc, id_set_backtrace, 1, bt); + rb_exc_raise(exc); +} + static void parser_error(const char *msg) { VALUE exc = rb_exc_new2(eHttpParserError, msg); - VALUE bt = rb_ary_new(); - rb_funcall(exc, rb_intern("set_backtrace"), 1, bt); - rb_exc_raise(exc); + raise_with_empty_bt(exc); +} + +static void raise_414(const char *msg) +{ + VALUE exc = rb_exc_new2(eRequestURITooLongError, msg); + + raise_with_empty_bt(exc); } #define REMAINING (unsigned long)(pe - p) @@ -301,7 +316,7 @@ static void write_value(struct http_parser *hp, action request_uri { VALUE str; - VALIDATE_MAX_LENGTH(LEN(mark, fpc), REQUEST_URI); + VALIDATE_MAX_URI_LENGTH(LEN(mark, fpc), REQUEST_URI); str = rb_hash_aset(hp->env, g_request_uri, STR_NEW(mark, fpc)); /* * "OPTIONS * HTTP/1.1\r\n" is a valid request, but we can't have '*' @@ -314,19 +329,19 @@ static void write_value(struct http_parser *hp, } } action fragment { - VALIDATE_MAX_LENGTH(LEN(mark, fpc), FRAGMENT); + VALIDATE_MAX_URI_LENGTH(LEN(mark, fpc), FRAGMENT); rb_hash_aset(hp->env, g_fragment, STR_NEW(mark, fpc)); } action start_query {MARK(start.query, fpc); } action query_string { - VALIDATE_MAX_LENGTH(LEN(start.query, fpc), QUERY_STRING); + VALIDATE_MAX_URI_LENGTH(LEN(start.query, fpc), QUERY_STRING); rb_hash_aset(hp->env, g_query_string, STR_NEW(start.query, fpc)); } action http_version { http_version(hp, PTR_TO(mark), LEN(mark, fpc)); } action request_path { VALUE val; - VALIDATE_MAX_LENGTH(LEN(mark, fpc), REQUEST_PATH); + VALIDATE_MAX_URI_LENGTH(LEN(mark, fpc), REQUEST_PATH); val = rb_hash_aset(hp->env, g_request_path, STR_NEW(mark, fpc)); /* rack says PATH_INFO must start with "/" or be empty */ @@ -883,6 +898,9 @@ void Init_unicorn_http(void) cHttpParser = rb_define_class_under(mUnicorn, "HttpParser", rb_cObject); eHttpParserError = rb_define_class_under(mUnicorn, "HttpParserError", rb_eIOError); + eRequestURITooLongError = + rb_define_class_under(mUnicorn, "RequestURITooLongError", + eHttpParserError); init_globals(); rb_define_alloc_func(cHttpParser, HttpParser_alloc); @@ -932,6 +950,7 @@ void Init_unicorn_http(void) SET_GLOBAL(g_content_length, "CONTENT_LENGTH"); SET_GLOBAL(g_http_connection, "CONNECTION"); id_clear = rb_intern("clear"); + id_set_backtrace = rb_intern("set_backtrace"); init_unicorn_httpdate(); } #undef SET_GLOBAL diff --git a/lib/unicorn/const.rb b/lib/unicorn/const.rb index 717a61f..fe49e1b 100644 --- a/lib/unicorn/const.rb +++ b/lib/unicorn/const.rb @@ -31,6 +31,7 @@ module Unicorn::Const # :stopdoc: # common errors we'll send back ERROR_400_RESPONSE = "HTTP/1.1 400 Bad Request\r\n\r\n" + ERROR_414_RESPONSE = "HTTP/1.1 414 Request-URI Too Long\r\n\r\n" ERROR_500_RESPONSE = "HTTP/1.1 500 Internal Server Error\r\n\r\n" EXPECT_100_RESPONSE = "HTTP/1.1 100 Continue\r\n\r\n" diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb index 3077b95..994de67 100644 --- a/lib/unicorn/http_server.rb +++ b/lib/unicorn/http_server.rb @@ -527,6 +527,8 @@ class Unicorn::HttpServer msg = case e when EOFError,Errno::ECONNRESET,Errno::EPIPE,Errno::EINVAL,Errno::EBADF Unicorn::Const::ERROR_500_RESPONSE + when Unicorn::RequestURITooLongError + Unicorn::Const::ERROR_414_RESPONSE when Unicorn::HttpParserError # try to tell the client they're bad Unicorn::Const::ERROR_400_RESPONSE else diff --git a/t/t0002-parser-error.sh b/t/t0002-parser-error.sh index 9a3e7cf..f0df69d 100755 --- a/t/t0002-parser-error.sh +++ b/t/t0002-parser-error.sh @@ -1,6 +1,6 @@ #!/bin/sh . ./test-lib.sh -t_plan 5 "parser error test" +t_plan 11 "parser error test" t_begin "setup and startup" && { unicorn_setup @@ -24,6 +24,66 @@ t_begin "response should be a 400" && { grep -F 'HTTP/1.1 400 Bad Request' $tmp } +t_begin "send a huge Request URI (REQUEST_PATH > (12 * 1024))" && { + rm -f $tmp + ( + cat $fifo > $tmp & + set -e + trap 'wait && echo ok > $ok' EXIT + printf 'GET /' + for i in $(awk $fifo || : + test xok = x$(cat $ok) +} + +t_begin "response should be a 414" && { + grep -F 'HTTP/1.1 414 Request-URI Too Long' $tmp +} + +t_begin "send a huge Request URI (QUERY_STRING > (10 * 1024))" && { + rm -f $tmp + ( + cat $fifo > $tmp & + set -e + trap 'wait && echo ok > $ok' EXIT + printf 'GET /hello-world?a' + for i in $(awk $fifo || : + test xok = x$(cat $ok) +} + +t_begin "response should be a 414" && { + grep -F 'HTTP/1.1 414 Request-URI Too Long' $tmp +} + +t_begin "send a huge Request URI (FRAGMENT > 1024)" && { + rm -f $tmp + ( + cat $fifo > $tmp & + set -e + trap 'wait && echo ok > $ok' EXIT + printf 'GET /hello-world#a' + for i in $(awk $fifo || : + test xok = x$(cat $ok) +} + +t_begin "response should be a 414" && { + grep -F 'HTTP/1.1 414 Request-URI Too Long' $tmp +} + t_begin "server stderr should be clean" && check_stderr t_begin "term signal sent" && kill $unicorn_pid -- cgit v1.2.3-24-ge0c7