Skip to content

Loading…

Ensure HTTP method handling is correct #29

Merged
merged 5 commits into from

2 participants

@davidjb

Several issues fixed:

  1. VERSION-CONTROL and BASELINE-CONTROL are recognised (hyphen, not underscore)
  2. PATCH requests are now recognised (weren't before)
  3. Various RFCs are now followed for accepting request bodies (input) -- in particular, various WebDAV methods saw their request bodies ignored before. (See RFC list below)
  4. Includes a QA test that covers all the above -- ensures that all methods can be submitted with or without input and the correct status code is returned.

See

http://www.webdav.org/specs/rfc2518.html
http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html
http://tools.ietf.org/html/rfc5789#section-2.1
http://www.webdav.org/specs/rfc3253.html
for RFC details.

@skinkie
Cherokee Project member

Thanks for this great effort David!

@skinkie skinkie merged commit 435910e into cherokee:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 147 additions and 14 deletions.
  1. +1 −1 cherokee/handler_cgi.c
  2. +2 −2 cherokee/handler_fcgi.c
  3. +4 −2 cherokee/header.c
  4. +4 −4 cherokee/http.c
  5. +18 −4 cherokee/http.h
  6. +1 −1 qa/298-Timeout.py
  7. +117 −0 qa/299-MethodsRequestBodyHandling.py
View
2 cherokee/handler_cgi.c
@@ -367,7 +367,7 @@ add_environment (cherokee_handler_cgi_t *cgi,
/* CONTENT_LENGTH
*/
- if (http_method_with_input (conn->header.method)) {
+ if (conn->post.has_info) {
cherokee_buffer_clean (tmp);
cherokee_buffer_add_ullong10 (tmp, conn->post.len);
set_env (cgi_base, "CONTENT_LENGTH", tmp->buf, tmp->len);
View
4 cherokee/handler_fcgi.c
@@ -404,7 +404,7 @@ add_extra_fcgi_env (cherokee_handler_fcgi_t *hdl, cuint_t *last_header_offset)
/* POST management
*/
- if (http_method_with_input (conn->header.method)) {
+ if (conn->post.has_info) {
if (conn->post.encoding == post_enc_regular) {
cherokee_buffer_add_ullong10 (&buffer, conn->post.len);
set_env (cgi_base, "CONTENT_LENGTH", buffer.buf, buffer.len);
@@ -516,7 +516,7 @@ build_header (cherokee_handler_fcgi_t *hdl, cherokee_buffer_t *buffer)
/* No POST?
*/
- if ((! http_method_with_input (conn->header.method)) || (! conn->post.has_info) || (! conn->post.len)) {
+ if ((! conn->post.has_info) || (! conn->post.len)) {
TRACE (ENTRIES",post", "Post: %s\n", "has no post");
add_empty_packet (hdl, FCGI_STDIN);
}
View
6 cherokee/header.c
@@ -321,6 +321,8 @@ parse_method (cherokee_header_t *hdr, char *line, char *end, char **pointer)
else
detect_method (line, "PURGE", purge)
else
+ detect_method (line, "PATCH", patch)
+ else
detect_method (line, "POLL", poll)
else
detect_method (line, "PROPFIND", propfind)
@@ -385,10 +387,10 @@ parse_method (cherokee_header_t *hdr, char *line, char *end, char **pointer)
detect_method (line, "REPORT", report)
break;
case 'V':
- detect_method (line, "VERSION_CONTROL", version_control)
+ detect_method (line, "VERSION-CONTROL", version_control)
break;
case 'B':
- detect_method (line, "BASELINE_CONTROL", baseline_control)
+ detect_method (line, "BASELINE-CONTROL", baseline_control)
break;
case 'I':
detect_method (line, "INVALID", invalid)
View
8 cherokee/http.c
@@ -65,7 +65,7 @@ cherokee_http_method_to_string (cherokee_http_method_t method, const char **str,
entry (http_unsubscribe, "UNSUBSCRIBE");
entry (http_report, "REPORT");
entry (http_patch, "PATCH");
- entry (http_version_control, "VERSION_CONTROL");
+ entry (http_version_control, "VERSION-CONTROL");
entry (http_checkout, "CHECKOUT");
entry (http_uncheckout, "UNCHECKOUT");
entry (http_checkin, "CHECKIN");
@@ -73,7 +73,7 @@ cherokee_http_method_to_string (cherokee_http_method_t method, const char **str,
entry (http_label, "LABEL");
entry (http_mkworkspace, "MKWORKSPACE");
entry (http_mkactivity, "MKACTIVITY");
- entry (http_baseline_control, "BASELINE_CONTROL");
+ entry (http_baseline_control, "BASELINE-CONTROL");
entry (http_merge, "MERGE");
entry (http_invalid, "INVALID");
@@ -135,7 +135,7 @@ cherokee_http_string_to_method (cherokee_buffer_t *string,
*method = http_report;
else if (cherokee_buffer_case_cmp_str (string, "patch") == 0)
*method = http_patch;
- else if (cherokee_buffer_case_cmp_str (string, "version_control") == 0)
+ else if (cherokee_buffer_case_cmp_str (string, "version-control") == 0)
*method = http_version_control;
else if (cherokee_buffer_case_cmp_str (string, "checkout") == 0)
*method = http_checkout;
@@ -151,7 +151,7 @@ cherokee_http_string_to_method (cherokee_buffer_t *string,
*method = http_mkworkspace;
else if (cherokee_buffer_case_cmp_str (string, "mkactivity") == 0)
*method = http_mkactivity;
- else if (cherokee_buffer_case_cmp_str (string, "baseline_control") == 0)
+ else if (cherokee_buffer_case_cmp_str (string, "baseline-control") == 0)
*method = http_baseline_control;
else if (cherokee_buffer_case_cmp_str (string, "merge") == 0)
*method = http_merge;
View
22 cherokee/http.h
@@ -224,18 +224,32 @@ typedef enum { /* Protocol RFC Section */
#define http_method_with_body(m) ((m) != http_head)
+/* RFC 2518, RFC 2616, RFC3253, RFC 5789 */
#define http_method_with_input(m) ((m == http_post) || \
(m == http_put) || \
- (m == http_mkcol) || \
(m == http_merge) || \
(m == http_search) || \
(m == http_report) || \
- (m == http_checkout) || \
+ (m == http_patch) || \
(m == http_propfind) || \
(m == http_proppatch) || \
- (m == http_mkactivity))
+ (m == http_update) || \
+ (m == http_label))
-#define http_method_with_optional_input(m) (m == http_options)
+#define http_method_with_optional_input(m) ((m == http_options) || \
+ (m == http_delete) || \
+ (m == http_mkcol) || \
+ (m == http_copy) || \
+ (m == http_move) || \
+ (m == http_lock) || \
+ (m == http_unlock) || \
+ (m == http_version_control) || \
+ (m == http_checkout) || \
+ (m == http_uncheckout) || \
+ (m == http_checkin) || \
+ (m == http_mkworkspace) || \
+ (m == http_mkactivity) || \
+ (m == http_baseline_control))
/* RFC 2616: Section 4.3 */
View
2 qa/298-Timeout.py
@@ -71,7 +71,7 @@ def JustBefore (self, www):
# Instant return and 1 second less than timeout should work,
# but past the timeout should return no content.
for dir, timeout in ((DIR, SERVER_TIMEOUT), (DIR_RULE, RULE_TIMEOUT)):
- for script_runtime in (0, timeout-1, timeout+1):
+ for script_runtime in (0, timeout-2, timeout+2):
# Write the new script files
filename = 'test-%i-seconds.cgi' % script_runtime
code = CGI_CODE % dict(runtime=script_runtime, content=CONTENT)
View
117 qa/299-MethodsRequestBodyHandling.py
@@ -0,0 +1,117 @@
+from base import *
+
+DIR = "299-MethodsRequestBodyHandling"
+MAGIC = "Report bugs to http://bugs.cherokee-project.com"
+
+METHODS = {
+ 'required': [
+ 'POST',
+ 'PUT',
+ 'MERGE',
+ 'SEARCH',
+ 'REPORT',
+ 'PATCH',
+ 'PROPFIND',
+ 'PROPPATCH',
+ 'UPDATE',
+ 'LABEL',
+ ],
+ 'optional': [
+ 'OPTIONS',
+ 'DELETE',
+ 'MKCOL',
+ 'COPY',
+ 'MOVE',
+ 'LOCK',
+ 'UNLOCK',
+ 'VERSION-CONTROL',
+ 'CHECKOUT',
+ 'UNCHECKOUT',
+ 'CHECKIN',
+ 'MKWORKSPACE',
+ 'MKACTIVITY',
+ 'BASELINE-CONTROL',
+ ]
+}
+
+CONF = """
+vserver!1!rule!1280!match = directory
+vserver!1!rule!1280!match!directory = /%s
+vserver!1!rule!1280!handler = common
+""" % (DIR)
+
+
+class TestEntry (TestBase):
+ """Test for HTTP methods with required and optional request bodies being
+ received and processed correctly by Cherokee.
+ """
+
+ def __init__ (self, method, send_input, input_required):
+ TestBase.__init__ (self, __file__)
+ self.request = "%s /%s/ HTTP/1.0\r\n" % (method, DIR) +\
+ "Content-type: text/xml\r\n"
+ self.expected_content = []
+
+ if input_required and not send_input:
+ self.expected_error = 411
+ else:
+ self.expected_error = 200
+ self.expected_content.append("Method: %s" % method)
+
+ if send_input:
+ self.request += "Content-length: %d\r\n" % (len(MAGIC))
+ self.post = MAGIC
+ self.expected_content.append("Body: %s" % MAGIC)
+ else:
+ self.forbidden_content = 'Body:'
+
+
+class Test (TestCollection):
+
+ def __init__ (self):
+ TestCollection.__init__ (self, __file__)
+
+ self.name = "Method Request Body Handling"
+ self.conf = CONF
+ self.proxy_suitable = True
+
+ def Prepare (self, www):
+ d = self.Mkdir (www, DIR)
+ f = self.WriteFile (d, "test_index.php", 0444,
+ """
+<?php echo 'Method: '.$_SERVER['REQUEST_METHOD']; ?>
+
+<?php
+ $body = @file_get_contents('php://input');
+ if (strlen($body) > 0):
+ echo "Body: $body";
+ endif;
+?>
+ """)
+
+ def JustBefore (self, www):
+ # Create sub-request objects
+ self.Empty ()
+
+ # Create all tests with different methods - all methods
+ # should only work with request bodies. With no request
+ # body a 411 Length Required should result.
+ for method in METHODS['required']:
+ self.Add (TestEntry (method=method,
+ send_input=True,
+ input_required=True))
+ self.Add (TestEntry (method=method,
+ send_input=False,
+ input_required=True))
+
+ # Create all tests with different methods - all methods
+ # should work with and without request bodies.
+ for method in METHODS['optional']:
+ #Test method when for when sending a request body
+ self.Add (TestEntry (method=method,
+ send_input=True,
+ input_required=False))
+ #Test method when not sending a request body
+ self.Add (TestEntry (method=method,
+ send_input=False,
+ input_required=False))
Something went wrong with that request. Please try again.