From ab221ddff81f042d30a421974b067533e39f1f23 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Wed, 2 Dec 2009 17:48:55 +0100
Subject: [PATCH] http.server: No longer enfore JSON encoding for body

The HTTP layer shouldn't care about the contents of the request data or
responses. This requires further changes in the RAPI code to handle client
requests correctly.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 NEWS                 |  7 +++++++
 daemons/ganeti-noded | 12 +++++++-----
 daemons/ganeti-rapi  | 30 ++++++++++++++++++++++++++++--
 lib/http/__init__.py | 11 +----------
 lib/http/server.py   | 10 +++++-----
 lib/rapi/baserlib.py | 14 +++++++-------
 lib/rapi/rlib2.py    | 10 +++++-----
 7 files changed, 60 insertions(+), 34 deletions(-)

diff --git a/NEWS b/NEWS
index ec983769b..a0eae07ee 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,13 @@
 News
 ====
 
+Version 2.2.0
+-------------
+
+- RAPI now requires a Content-Type header for requests with a body (e.g.
+  ``PUT`` or ``POST``) which must be set to ``application/json`` (see
+  RFC2616 (HTTP/1.1), section 7.2.1)
+
 
 Version 2.1.0
 -------------
diff --git a/daemons/ganeti-noded b/daemons/ganeti-noded
index d91eef4c7..4d27e180d 100755
--- a/daemons/ganeti-noded
+++ b/daemons/ganeti-noded
@@ -45,6 +45,7 @@ from ganeti import daemon
 from ganeti import http
 from ganeti import utils
 from ganeti import storage
+from ganeti import serializer
 
 import ganeti.http.server # pylint: disable-msg=W0611
 
@@ -99,14 +100,13 @@ class NodeHttpServer(http.server.HttpServer):
       raise http.HttpNotFound()
 
     try:
-      rvalue = method(req.request_body)
-      return True, rvalue
+      result = (True, method(serializer.LoadJson(req.request_body)))
 
     except backend.RPCFail, err:
       # our custom failure exception; str(err) works fine if the
       # exception was constructed with a single argument, and in
       # this case, err.message == err.args[0] == str(err)
-      return (False, str(err))
+      result = (False, str(err))
     except errors.QuitGanetiException, err:
       # Tell parent to quit
       logging.info("Shutting down the node daemon, arguments: %s",
@@ -114,10 +114,12 @@ class NodeHttpServer(http.server.HttpServer):
       os.kill(self.noded_pid, signal.SIGTERM)
       # And return the error's arguments, which must be already in
       # correct tuple format
-      return err.args
+      result = err.args
     except Exception, err:
       logging.exception("Error in RPC call")
-      return False, "Error while executing backend function: %s" % str(err)
+      result = (False, "Error while executing backend function: %s" % str(err))
+
+    return serializer.DumpJson(result, indent=False)
 
   # the new block devices  --------------------------
 
diff --git a/daemons/ganeti-rapi b/daemons/ganeti-rapi
index 00654e1df..b2a275386 100755
--- a/daemons/ganeti-rapi
+++ b/daemons/ganeti-rapi
@@ -52,13 +52,14 @@ class RemoteApiRequestContext(object):
     self.handler = None
     self.handler_fn = None
     self.handler_access = None
+    self.body_data = None
 
 
 class JsonErrorRequestExecutor(http.server.HttpServerRequestExecutor):
   """Custom Request Executor class that formats HTTP errors in JSON.
 
   """
-  error_content_type = "application/json"
+  error_content_type = http.HttpJsonConverter.CONTENT_TYPE
 
   def _FormatErrorMessage(self, values):
     """Formats the body of an error message.
@@ -116,6 +117,9 @@ class RemoteApiHttpServer(http.auth.HttpServerRequestAuthentication,
       if ctx.handler_access is None:
         raise AssertionError("Permissions definition missing")
 
+      # This is only made available in HandleRequest
+      ctx.body_data = None
+
       req.private = ctx
 
     return req.private
@@ -162,6 +166,25 @@ class RemoteApiHttpServer(http.auth.HttpServerRequestAuthentication,
     """
     ctx = self._GetRequestContext(req)
 
+    # Deserialize request parameters
+    if req.request_body:
+      # RFC2616, 7.2.1: Any HTTP/1.1 message containing an entity-body SHOULD
+      # include a Content-Type header field defining the media type of that
+      # body. [...] If the media type remains unknown, the recipient SHOULD
+      # treat it as type "application/octet-stream".
+      req_content_type = req.request_headers.get(http.HTTP_CONTENT_TYPE,
+                                                 http.HTTP_APP_OCTET_STREAM)
+      if (req_content_type.lower() !=
+          http.HttpJsonConverter.CONTENT_TYPE.lower()):
+        raise http.HttpUnsupportedMediaType()
+
+      try:
+        ctx.body_data = serializer.LoadJson(req.request_body)
+      except Exception:
+        raise http.HttpBadRequest(message="Unable to parse JSON data")
+    else:
+      ctx.body_data = None
+
     try:
       result = ctx.handler_fn()
     except luxi.TimeoutError:
@@ -173,7 +196,10 @@ class RemoteApiHttpServer(http.auth.HttpServerRequestAuthentication,
       logging.exception("Error while handling the %s request", method)
       raise
 
-    return result
+    req.resp_headers[http.HTTP_CONTENT_TYPE] = \
+      http.HttpJsonConverter.CONTENT_TYPE
+
+    return serializer.DumpJson(result)
 
 
 def CheckRapi(options, args):
diff --git a/lib/http/__init__.py b/lib/http/__init__.py
index 272cff0e9..b2917152b 100644
--- a/lib/http/__init__.py
+++ b/lib/http/__init__.py
@@ -680,7 +680,6 @@ class HttpMessage(object):
     self.start_line = None
     self.headers = None
     self.body = None
-    self.decoded_body = None
 
 
 class HttpClientToServerStartLine(object):
@@ -851,17 +850,9 @@ class HttpMessageReader(object):
     assert self.parser_status == self.PS_COMPLETE
     assert not buf, "Parser didn't read full response"
 
+    # Body is complete
     msg.body = self.body_buffer.getvalue()
 
-    # TODO: Content-type, error handling
-    if msg.body:
-      msg.decoded_body = HttpJsonConverter().Decode(msg.body)
-    else:
-      msg.decoded_body = None
-
-    if msg.decoded_body:
-      logging.debug("Message body: %s", msg.decoded_body)
-
   def _ContinueParsing(self, buf, eof):
     """Main function for HTTP message state machine.
 
diff --git a/lib/http/server.py b/lib/http/server.py
index fbc7ec7fa..e395030de 100644
--- a/lib/http/server.py
+++ b/lib/http/server.py
@@ -78,7 +78,7 @@ class _HttpServerRequest(object):
     self.request_method = request_msg.start_line.method
     self.request_path = request_msg.start_line.path
     self.request_headers = request_msg.headers
-    self.request_body = request_msg.decoded_body
+    self.request_body = request_msg.body
 
     # Response attributes
     self.resp_headers = {}
@@ -333,12 +333,12 @@ class HttpServerRequestExecutor(object):
         logging.exception("Unknown exception")
         raise http.HttpInternalServerError(message="Unknown error")
 
-      # TODO: Content-type
-      encoder = http.HttpJsonConverter()
+      if not isinstance(result, basestring):
+        raise http.HttpError("Handler function didn't return string type")
+
       self.response_msg.start_line.code = http.HTTP_OK
-      self.response_msg.body = encoder.Encode(result)
       self.response_msg.headers = handler_context.resp_headers
-      self.response_msg.headers[http.HTTP_CONTENT_TYPE] = encoder.CONTENT_TYPE
+      self.response_msg.body = result
     finally:
       # No reason to keep this any longer, even for exceptions
       handler_context.private = None
diff --git a/lib/rapi/baserlib.py b/lib/rapi/baserlib.py
index 1f23f43bb..09d594c70 100644
--- a/lib/rapi/baserlib.py
+++ b/lib/rapi/baserlib.py
@@ -272,13 +272,13 @@ class R_Generic(object):
     @param name: the required parameter
 
     """
-    if name in self.req.request_body:
-      return self.req.request_body[name]
-    elif args:
-      return args[0]
-    else:
-      raise http.HttpBadRequest("Required parameter '%s' is missing" %
-                                name)
+    try:
+      return self.req.private.body_data[name]
+    except KeyError:
+      if args:
+        return args[0]
+
+    raise http.HttpBadRequest("Required parameter '%s' is missing" % name)
 
   def useLocking(self):
     """Check if the request specifies locking.
diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
index f47527757..d215736d0 100644
--- a/lib/rapi/rlib2.py
+++ b/lib/rapi/rlib2.py
@@ -256,11 +256,11 @@ class R_2_nodes_name_role(baserlib.R_Generic):
     @return: a job id
 
     """
-    if not isinstance(self.req.request_body, basestring):
+    if not isinstance(self.req.private.body_data, basestring):
       raise http.HttpBadRequest("Invalid body contents, not a string")
 
     node_name = self.items[0]
-    role = self.req.request_body
+    role = self.req.private.body_data
 
     if role == _NR_REGULAR:
       candidate = False
@@ -431,12 +431,12 @@ class R_2_instances(baserlib.R_Generic):
     @return: a job id
 
     """
-    if not isinstance(self.req.request_body, dict):
+    if not isinstance(self.req.private.body_data, dict):
       raise http.HttpBadRequest("Invalid body contents, not a dictionary")
 
-    beparams = baserlib.MakeParamsDict(self.req.request_body,
+    beparams = baserlib.MakeParamsDict(self.req.private.body_data,
                                        constants.BES_PARAMETERS)
-    hvparams = baserlib.MakeParamsDict(self.req.request_body,
+    hvparams = baserlib.MakeParamsDict(self.req.private.body_data,
                                        constants.HVS_PARAMETERS)
     fn = self.getBodyParameter
 
-- 
GitLab