dovecot-2.2: lib-http: Adjusted message and request parsers to r...

dovecot at dovecot.org dovecot at dovecot.org
Sun Sep 15 03:50:34 EEST 2013


details:   http://hg.dovecot.org/dovecot-2.2/rev/28e5d58e49dc
changeset: 16749:28e5d58e49dc
user:      Stephan Bosch <stephan at rename-it.nl>
date:      Sun Sep 15 03:50:08 2013 +0300
description:
lib-http: Adjusted message and request parsers to return an error code.

diffstat:

 src/lib-http/http-message-parser.c  |   91 +++++++++++-----
 src/lib-http/http-message-parser.h  |   21 ++-
 src/lib-http/http-request-parser.c  |  117 ++++++++++++++++-----
 src/lib-http/http-request-parser.h  |   13 ++-
 src/lib-http/http-response-parser.c |  189 ++++++++++++++++++-----------------
 src/lib-http/test-http-server.c     |    3 +-
 6 files changed, 277 insertions(+), 157 deletions(-)

diffs (truncated from 815 to 300 lines):

diff -r eeaa68773f73 -r 28e5d58e49dc src/lib-http/http-message-parser.c
--- a/src/lib-http/http-message-parser.c	Sun Sep 15 03:47:54 2013 +0300
+++ b/src/lib-http/http-message-parser.c	Sun Sep 15 03:50:08 2013 +0300
@@ -62,35 +62,45 @@
 	const unsigned char *p = parser->cur;
 	const size_t size = parser->end - parser->cur;
 
+	parser->error_code = HTTP_MESSAGE_PARSE_ERROR_NONE;
+	parser->error = NULL;
+
 	/* HTTP-version  = HTTP-name "/" DIGIT "." DIGIT
 	   HTTP-name     = %x48.54.54.50 ; "HTTP", case-sensitive
 	 */
 	if (size < 8)
 		return 0;
 	if (memcmp(p, "HTTP/", 5) != 0 ||
-	    !i_isdigit(p[5]) || p[6] != '.' || !i_isdigit(p[7]))
+	    !i_isdigit(p[5]) || p[6] != '.' || !i_isdigit(p[7])) {
+		parser->error = "Bad HTTP version";
+		parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE;
 		return -1;
+	}
 	parser->msg.version_major = p[5] - '0';
 	parser->msg.version_minor = p[7] - '0';
 	parser->cur += 8;
 	return 1;
 }
 
-int http_message_parse_finish_payload(struct http_message_parser *parser,
-				      const char **error_r)
+int http_message_parse_finish_payload(struct http_message_parser *parser)
 {
 	const unsigned char *data;
 	size_t size;
 	int ret;
 
+	parser->error_code = HTTP_MESSAGE_PARSE_ERROR_NONE;
+	parser->error = NULL;
+
 	if (parser->payload == NULL)
 		return 1;
 
 	while ((ret = i_stream_read_data(parser->payload, &data, &size, 0)) > 0)
 		i_stream_skip(parser->payload, size);
 	if (ret == 0 || parser->payload->stream_errno != 0) {
-		if (ret < 0)
-			*error_r = "Stream error while skipping payload";
+		if (ret < 0) {
+			parser->error = "Stream error while skipping payload";
+			parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_STREAM;
+		}
 		return ret;
 	}
 	i_stream_unref(&parser->payload);
@@ -99,8 +109,7 @@
 
 static int
 http_message_parse_header(struct http_message_parser *parser,
-			  const char *name, const unsigned char *data, size_t size,
-			  const char **error_r)
+			  const char *name, const unsigned char *data, size_t size)
 {
 	const struct http_header_field *hdr;
 	struct http_parser hparser;
@@ -141,7 +150,8 @@
 			}
 
 			if (hparser.cur < hparser.end || num_tokens == 0) {
-				*error_r = "Invalid Connection header";
+				parser->error = "Invalid Connection header";
+				parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE;
 				return -1;
 			}
 
@@ -150,12 +160,14 @@
 		/* Content-Length: */
 		if (strcasecmp(name, "Content-Length") == 0) {
 			if (parser->msg.have_content_length) {
-				*error_r = "Duplicate Content-Length header";
+				parser->error = "Duplicate Content-Length header";
+				parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE;
 				return -1;
 			}
 			/* Content-Length = 1*DIGIT */
 			if (str_to_uoff(hdr->value, &parser->msg.content_length) < 0) {
-				*error_r = "Invalid Content-Length header";
+				parser->error= "Invalid Content-Length header";
+				parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE;
 				return -1;
 			}
 			parser->msg.have_content_length = TRUE;
@@ -165,7 +177,8 @@
 	case 'D': case 'd':
 		if (strcasecmp(name, "Date") == 0) {
 			if (parser->msg.date != (time_t)-1) {
-				*error_r = "Duplicate Date header";
+				parser->error = "Duplicate Date header";
+				parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE;
 				return -1;
 			}
 
@@ -269,7 +282,8 @@
 
 			if (hparser.cur < hparser.end ||
 				array_count(&parser->msg.transfer_encoding) == 0) {
-				*error_r = "Invalid Transfer-Encoding header";
+				parser->error = "Invalid Transfer-Encoding header";
+				parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE;
 				return -1;
 			}
 			return 0;
@@ -281,14 +295,16 @@
 	return 0;
 }
 
-int http_message_parse_headers(struct http_message_parser *parser,
-			       const char **error_r)
+int http_message_parse_headers(struct http_message_parser *parser)
 {
 	const unsigned char *field_data;
 	const char *field_name, *error;
 	size_t field_size;
 	int ret;
 
+	parser->error_code = HTTP_MESSAGE_PARSE_ERROR_NONE;
+	parser->error = NULL;
+
 	/* *( header-field CRLF ) CRLF */
 	while ((ret=http_header_parse_next_field(parser->header_parser,
 		&field_name, &field_data, &field_size, &error)) > 0) {
@@ -298,23 +314,28 @@
 		}
 
 		if (http_message_parse_header(parser,
-			field_name, field_data, field_size, error_r) < 0)
+			field_name, field_data, field_size) < 0)
 			return -1;
 	}
 
 	if (ret < 0) {
-		*error_r = t_strdup_printf(
-			"Failed to parse response header: %s", error);
+		if (parser->input->eof || parser->input->stream_errno != 0)  {			
+			parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_STREAM;
+			parser->error = "Broken stream";
+		} else {
+			parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE;
+			parser->error = t_strdup_printf("Failed to parse header: %s", error);
+		}
+	
 	}
 	return ret;
 }
 
-
-int http_message_parse_body(struct http_message_parser *parser, bool request,
-			    const char **error_r)
+int http_message_parse_body(struct http_message_parser *parser, bool request)
 {
-	*error_r = NULL;
-
+	parser->error_code = HTTP_MESSAGE_PARSE_ERROR_NONE;
+	parser->error = NULL;
+ 
 	if (array_is_created(&parser->msg.transfer_encoding)) {
 		const struct http_transfer_coding *coding;
 
@@ -324,20 +345,27 @@
 			if (strcasecmp(coding->name, "chunked") == 0) {
 				chunked_last = TRUE;
 		
-				if (*error_r == NULL && array_is_created(&coding->parameters) &&
-					array_count(&coding->parameters) > 0) {
+				if ((parser->error_code == HTTP_MESSAGE_PARSE_ERROR_NONE)
+					&& array_is_created(&coding->parameters)
+					&& array_count(&coding->parameters) > 0) {
 					const struct http_transfer_param *param =
 						array_idx(&coding->parameters, 0);
-					
-					*error_r = t_strdup_printf("Unexpected parameter `%s' specified"
+
+					parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BAD_MESSAGE;
+					parser->error = t_strdup_printf(
+						"Unexpected parameter `%s' specified"
 						"for the `%s' transfer coding", param->attribute, coding->name);
+					/* recoverable */
 				}
 			} else if (chunked_last) {
-				*error_r = "Chunked Transfer-Encoding must be last";
+				parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE;
+				parser->error = "Chunked Transfer-Encoding must be last";
 				return -1;
-			} else if (*error_r == NULL) {
- 				*error_r = t_strdup_printf(
+			} else if (parser->error_code == HTTP_MESSAGE_PARSE_ERROR_NONE) {
+				parser->error_code = HTTP_MESSAGE_PARSE_ERROR_NOT_IMPLEMENTED;
+				parser->error = t_strdup_printf(
   				"Unknown transfer coding `%s'", coding->name);
+				/* recoverable */
   		}
   	}
 
@@ -364,7 +392,8 @@
 			   length cannot be determined reliably; the server MUST respond with
 			   the 400 (Bad Request) status code and then close the connection.
 			 */
-			*error_r = "Final Transfer-Encoding in request is not `chunked'";
+			parser->error_code = HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE;
+			parser->error = "Final Transfer-Encoding in request is not chunked";
 			return -1;
 		}
 
@@ -401,5 +430,7 @@
 		parser->payload =
 			i_stream_create_limit(parser->input, (size_t)-1);
 	}
+	if (parser->error_code != HTTP_MESSAGE_PARSE_ERROR_NONE)
+		return -1;
 	return 0;
 }
diff -r eeaa68773f73 -r 28e5d58e49dc src/lib-http/http-message-parser.h
--- a/src/lib-http/http-message-parser.h	Sun Sep 15 03:47:54 2013 +0300
+++ b/src/lib-http/http-message-parser.h	Sun Sep 15 03:50:08 2013 +0300
@@ -6,6 +6,15 @@
 
 #include "http-header.h"
 
+enum http_message_parse_error {
+	HTTP_MESSAGE_PARSE_ERROR_NONE = 0,        /* no error */
+	HTTP_MESSAGE_PARSE_ERROR_BROKEN_STREAM,   /* stream error */
+	HTTP_MESSAGE_PARSE_ERROR_BROKEN_MESSAGE,  /* unrecoverable generic error */
+	HTTP_MESSAGE_PARSE_ERROR_BAD_MESSAGE,     /* recoverable generic error */
+	HTTP_MESSAGE_PARSE_ERROR_NOT_IMPLEMENTED, /* used unimplemented feature
+	                                            (recoverable) */
+};
+
 struct http_message {
 	pool_t pool;
 
@@ -30,6 +39,9 @@
 
 	const unsigned char *cur, *end;
 
+	const char *error;
+	enum http_message_parse_error error_code;
+
 	struct http_header_parser *header_parser;
 	struct istream *payload;
 
@@ -44,12 +56,9 @@
 void http_message_parser_restart(struct http_message_parser *parser,
 	pool_t pool);
 
-int http_message_parse_finish_payload(struct http_message_parser *parser,
-				      const char **error_r);
+int http_message_parse_finish_payload(struct http_message_parser *parser);
 int http_message_parse_version(struct http_message_parser *parser);
-int http_message_parse_headers(struct http_message_parser *parser,
-			       const char **error_r);
-int http_message_parse_body(struct http_message_parser *parser, bool request,
-			    const char **error_r);
+int http_message_parse_headers(struct http_message_parser *parser);
+int http_message_parse_body(struct http_message_parser *parser, bool request);
 
 #endif
diff -r eeaa68773f73 -r 28e5d58e49dc src/lib-http/http-request-parser.c
--- a/src/lib-http/http-request-parser.c	Sun Sep 15 03:47:54 2013 +0300
+++ b/src/lib-http/http-request-parser.c	Sun Sep 15 03:50:08 2013 +0300
@@ -24,6 +24,8 @@
 	struct http_message_parser parser;
 	enum http_request_parser_state state;
 
+	enum http_request_parse_error error_code;
+
 	const char *request_method;
 	const char *request_target;
 
@@ -106,7 +108,7 @@
 }
 
 static int http_request_parse(struct http_request_parser *parser,
-			      pool_t pool, const char **error_r)
+			      pool_t pool)
 {
 	struct http_message_parser *_parser = &parser->parser;
 	int ret;
@@ -125,7 +127,8 @@
 			if (*_parser->cur == '\r' || *_parser->cur == '\n') {
 				if (parser->skipping_line) {
 					/* second extra CRLF; not allowed */
-					*error_r = "Empty request line";
+					parser->error_code = HTTP_REQUEST_PARSE_ERROR_BROKEN_REQUEST;
+					_parser->error = "Empty request line";
 					return -1;
 				}
 				/* HTTP/1.0 client sent one extra CRLF after body.
@@ -138,19 +141,17 @@
 			parser->skipping_line = FALSE;
 			/* fall through */
 		case HTTP_REQUEST_PARSE_STATE_METHOD:


More information about the dovecot-cvs mailing list