nicer TALER_MINT_parse_post_json return value handling, fixing return values where GNUNET_SYSERR is used instead of MHD_NO, marking cases where we should return a proper error message

src/mint/taler-mint-httpd_deposit.c

@@ -90,20 +90,18 @@ TALER_MINT_handler_deposit (struct RequestHandler *rh,
 
   res = TALER_MINT_parse_post_json (connection,
                                     connection_cls,
-                           upload_data, upload_data_size,
+                                    upload_data,
+                                    upload_data_size,
                                     &json);
   if (GNUNET_SYSERR == res)
-  {
-    // FIXME: return 'internal error'
-    GNUNET_break (0);
     return MHD_NO;
-  }
+  if ( (GNUNET_NO == res) || (NULL == json) )
-  if (GNUNET_NO == res)
     return MHD_YES;
   if (NULL == (db_conn = TALER_MINT_DB_get_connection ()))
   {
+    /* FIXME: return error message to client via MHD! */
     GNUNET_break (0);
-    return GNUNET_SYSERR;
+    return MHD_NO;
   }
 
   deposit = NULL;
@@ -213,7 +211,8 @@ TALER_MINT_handler_deposit (struct RequestHandler *rh,
     if (GNUNET_SYSERR == res)
     {
       GNUNET_break (0);
-      return GNUNET_SYSERR;
+    /* FIXME: return error message to client via MHD! */
+      return MHD_NO;
     }
   }
 
@@ -235,7 +234,8 @@ TALER_MINT_handler_deposit (struct RequestHandler *rh,
     if (GNUNET_SYSERR == res)
     {
       GNUNET_break (0);
-      return GNUNET_SYSERR;
+    /* FIXME: return error message to client via MHD! */
+      return MHD_NO;
     }
 
     /* coin valid but not known => insert into DB */
@@ -246,14 +246,16 @@ TALER_MINT_handler_deposit (struct RequestHandler *rh,
     if (GNUNET_OK != TALER_MINT_DB_insert_known_coin (db_conn, &known_coin))
     {
       GNUNET_break (0);
-      return GNUNET_SYSERR;
+    /* FIXME: return error message to client via MHD! */
+      return MHD_NO;
     }
   }
 
   if (GNUNET_OK != TALER_MINT_DB_insert_deposit (db_conn, deposit))
   {
     GNUNET_break (0);
-    return GNUNET_SYSERR;
+    /* FIXME: return error message to client via MHD! */
+    return MHD_NO;
   }
   return helper_deposit_send_response_success (connection, deposit);
 

src/mint/taler-mint-httpd_parsing.c

@@ -112,7 +112,6 @@ buffer_deinit (struct Buffer *buf)
  * @param max_size maximum size that the buffer can grow to
  * @return GNUNET_OK on success,
  *         GNUNET_NO if the buffer can't accomodate for the new data
- *         GNUNET_SYSERR on fatal error (out of memory?)
  */
 static int
 buffer_append (struct Buffer *buf,
@@ -142,18 +141,28 @@ buffer_append (struct Buffer *buf,
 
 
 /**
- * Process a POST request containing a JSON object.
+ * Process a POST request containing a JSON object.  This
+ * function realizes an MHD POST processor that will
+ * (incrementally) process JSON data uploaded to the HTTP
+ * server.  It will store the required state in the
+ * "connection_cls", which must be cleaned up using
+ * #TALER_MINT_parse_post_cleanup_callback().
  *
  * @param connection the MHD connection
- * @param con_cs the closure (contains a 'struct Buffer *')
+ * @param con_cs the closure (points to a `struct Buffer *`)
  * @param upload_data the POST data
- * @param upload_data_size the POST data size
+ * @param upload_data_size number of bytes in @a upload_data
  * @param json the JSON object for a completed request
- *
  * @returns
- *    GNUNET_YES if json object was parsed
+ *    GNUNET_YES if json object was parsed or at least
+ *               may be parsed in the future (call again);
+ *               `*json` will be NULL if we need to be called again,
+ *                and non-NULL if we are done.
  *    GNUNET_NO is request incomplete or invalid
+ *               (error message was generated)
  *    GNUNET_SYSERR on internal error
+ *               (we could not even queue an error message,
+ *                close HTTP session with MHD_NO)
  */
 int
 TALER_MINT_parse_post_json (struct MHD_Connection *connection,
@@ -164,10 +173,10 @@ TALER_MINT_parse_post_json (struct MHD_Connection *connection,
 {
   struct Buffer *r = *con_cls;
 
+  *json = NULL;
   if (NULL == *con_cls)
   {
     /* We are seeing a fresh POST request. */
-
     r = GNUNET_new (struct Buffer);
     if (GNUNET_OK !=
         buffer_init (r,
@@ -179,11 +188,15 @@ TALER_MINT_parse_post_json (struct MHD_Connection *connection,
       *con_cls = NULL;
       buffer_deinit (r);
       GNUNET_free (r);
-      return GNUNET_SYSERR;
+      return (MHD_NO ==
+              TALER_MINT_reply_internal_error (connection,
+                                               "out of memory"))
+        ? GNUNET_SYSERR : GNUNET_NO;
     }
+    /* everything OK, wait for more POST data */
     *upload_data_size = 0;
     *con_cls = r;
-    return GNUNET_NO;
+    return GNUNET_YES;
   }
   if (0 != *upload_data_size)
   {
@@ -195,31 +208,33 @@ TALER_MINT_parse_post_json (struct MHD_Connection *connection,
                        *upload_data_size,
                        REQUEST_BUFFER_MAX))
     {
-      /* Request too long or we're out of memory. */
+      /* Request too long */
-
       *con_cls = NULL;
       buffer_deinit (r);
       GNUNET_free (r);
-      return GNUNET_SYSERR;
+      return (MHD_NO ==
+              TALER_MINT_reply_request_too_large (connection))
+        ? GNUNET_SYSERR : GNUNET_NO;
     }
+    /* everything OK, wait for more POST data */
     *upload_data_size = 0;
-    return GNUNET_NO;
+    return GNUNET_YES;
   }
 
   /* We have seen the whole request. */
 
-  *json = json_loadb (r->data, r->fill, 0, NULL);
+  *json = json_loadb (r->data,
+                      r->fill,
+                      0,
+                      NULL);
   buffer_deinit (r);
   GNUNET_free (r);
   if (NULL == *json)
   {
     GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                "Can't parse JSON request body\n");
+                "Failed to parse JSON request body\n");
     return (MHD_YES ==
-            TALER_MINT_reply_json_pack (connection,
+            TALER_MINT_reply_invalid_json (connection))
-                                        MHD_HTTP_BAD_REQUEST,
-                                        "{s:s}",
-                                        "error", "invalid json"))
       ? GNUNET_NO : GNUNET_SYSERR;
   }
   *con_cls = NULL;

src/mint/taler-mint-httpd_parsing.h

@@ -80,10 +80,11 @@ enum
  * @param upload_data the POST data
  * @param upload_data_size number of bytes in @a upload_data
  * @param json the JSON object for a completed request
- *
  * @returns
  *    GNUNET_YES if json object was parsed or at least
- *               may be parsed in the future (call again)
+ *               may be parsed in the future (call again);
+ *               `*json` will be NULL if we need to be called again,
+ *                and non-NULL if we are done.
  *    GNUNET_NO is request incomplete or invalid
  *               (error message was generated)
  *    GNUNET_SYSERR on internal error

src/mint/taler-mint-httpd_refresh.c

@@ -609,18 +609,18 @@ TALER_MINT_handler_refresh_melt (struct RequestHandler *rh,
   res = TALER_MINT_parse_post_json (connection,
                                     connection_cls,
                                     upload_data,
-                           upload_data_size, &root);
+                                    upload_data_size,
+                                    &root);
   if (GNUNET_SYSERR == res)
-  {
-    // FIXME: return 'internal error'?
-    GNUNET_break (0);
     return MHD_NO;
-  }
+  if ( (GNUNET_NO == res) || (NULL == root) )
-  if (GNUNET_NO == res)
     return MHD_YES;
 
   if (NULL == (db_conn = TALER_MINT_DB_get_connection ()))
-    return GNUNET_SYSERR;
+  {
+    /* FIXME: return error code to MHD! */
+    return MHD_NO;
+  }
 
   /* session_pub field must always be present */
   res = request_json_require_nav (connection, root,
@@ -826,17 +826,13 @@ TALER_MINT_handler_refresh_commit (struct RequestHandler *rh,
   res = TALER_MINT_parse_post_json (connection,
                                     connection_cls,
                                     upload_data,
-                           upload_data_size, &root);
+                                    upload_data_size,
+                                    &root);
   if (GNUNET_SYSERR == res)
-  {
-    // FIXME: return 'internal error'?
-    GNUNET_break (0);
     return MHD_NO;
-  }
+  if ( (GNUNET_NO == res) || (NULL == root) )
-  if (GNUNET_NO == res)
     return MHD_YES;
 
-
   res = request_json_require_nav (connection, root,
                                   JNAV_FIELD, "session_pub",
                                   JNAV_RET_DATA,
@@ -1161,15 +1157,12 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
 
   res = TALER_MINT_parse_post_json (connection,
                                     connection_cls,
-                           upload_data, upload_data_size,
+                                    upload_data,
+                                    upload_data_size,
                                     &root);
   if (GNUNET_SYSERR == res)
-  {
-    // FIXME: return 'internal error'?
-    GNUNET_break (0);
     return MHD_NO;
-  }
+  if ( (GNUNET_NO == res) || (NULL == root) )
-  if (GNUNET_NO == res)
     return MHD_YES;
 
   res = request_json_require_nav (connection, root,
@@ -1186,6 +1179,7 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
   if (NULL == (db_conn = TALER_MINT_DB_get_connection ()))
   {
     GNUNET_break (0);
+    // FIXME: return 'internal error'?
     return MHD_NO;
   }
 
@@ -1199,6 +1193,7 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
   if (GNUNET_SYSERR == res)
   {
     GNUNET_break (0);
+    // FIXME: return 'internal error'?
     return MHD_NO;
   }
 
@@ -1242,14 +1237,16 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
       if (GNUNET_OK != res)
       {
         GNUNET_break (0);
-        return GNUNET_SYSERR;
+            // FIXME: return 'internal error'?
+        return MHD_NO;
       }
 
       res = TALER_MINT_DB_get_refresh_melt (db_conn, &refresh_session_pub, j, &coin_pub);
       if (GNUNET_OK != res)
       {
         GNUNET_break (0);
-        return GNUNET_SYSERR;
+        // FIXME: return 'internal error'?
+        return MHD_NO;
       }
 
       /* We're converting key types here, which is not very nice
@@ -1259,14 +1256,16 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
                                                &transfer_secret))
       {
         GNUNET_break (0);
-        return GNUNET_SYSERR;
+        // FIXME: return 'internal error'?
+        return MHD_NO;
       }
 
       if (0 >= TALER_refresh_decrypt (commit_link.shared_secret_enc, TALER_REFRESH_SHARED_SECRET_LENGTH,
                                       &transfer_secret, &shared_secret))
       {
         GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "decryption failed\n");
-        return GNUNET_SYSERR;
+        // FIXME: return 'internal error'?
+        return MHD_NO;
       }
 
       if (GNUNET_NO == secret_initialized)
@@ -1277,7 +1276,8 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
       else if (0 != memcmp (&shared_secret, &last_shared_secret, sizeof (struct GNUNET_HashCode)))
       {
         GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "shared secrets do not match\n");
-        return GNUNET_SYSERR;
+        // FIXME: return error code!
+        return MHD_NO;
       }
 
       {
@@ -1286,7 +1286,8 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
         if (0 != memcmp (&transfer_pub_check, &commit_link.transfer_pub, sizeof (struct GNUNET_CRYPTO_EcdsaPublicKey)))
         {
           GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "transfer keys do not match\n");
-          return GNUNET_SYSERR;
+        // FIXME: return error code!
+          return MHD_NO;
         }
       }
     }
@@ -1310,7 +1311,8 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
       if (GNUNET_OK != res)
       {
         GNUNET_break (0);
-        return GNUNET_SYSERR;
+                // FIXME: return error code!
+        return MHD_NO;
       }
 
 
@@ -1318,20 +1320,23 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
                                       &last_shared_secret, &link_data))
       {
         GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "decryption failed\n");
-        return GNUNET_SYSERR;
+                // FIXME: return error code!
+        return MHD_NO;
       }
 
       GNUNET_CRYPTO_ecdsa_key_get_public (&link_data.coin_priv, &coin_pub);
       if (NULL == (bkey = TALER_RSA_blinding_key_decode (&link_data.bkey_enc)))
       {
         GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "Invalid blinding key\n");
-        return GNUNET_SYSERR;
+                        // FIXME: return error code!
+        return MHD_NO;
       }
       res = TALER_MINT_DB_get_refresh_order (db_conn, j, &refresh_session_pub, &denom_pub);
       if (GNUNET_OK != res)
       {
         GNUNET_break (0);
-        return GNUNET_SYSERR;
+          // FIXME: return error code!
+        return MHD_NO;
       }
       if (NULL == (coin_ev_check =
                    TALER_RSA_message_blind (&coin_pub,
@@ -1340,7 +1345,8 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
                                             &denom_pub)))
       {
         GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "blind failed\n");
-        return GNUNET_SYSERR;
+          // FIXME: return error code!
+        return MHD_NO;
       }
 
       if (0 != memcmp (&coin_ev_check,
@@ -1349,7 +1355,8 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
       {
         GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "blind envelope does not match for kappa=%d, old=%d\n",
                     (int) i, (int) j);
-        return GNUNET_SYSERR;
+        // FIXME: return error code!
+        return MHD_NO;
       }
     }
   }
@@ -1358,6 +1365,7 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
   if (GNUNET_OK != TALER_MINT_DB_transaction (db_conn))
   {
     GNUNET_break (0);
+            // FIXME: return error code!
     return MHD_NO;
   }
 
@@ -1376,13 +1384,15 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
     if (GNUNET_OK != res)
     {
       GNUNET_break (0);
-      return GNUNET_SYSERR;
+              // FIXME: return error code!
+      return MHD_NO;
     }
     res = TALER_MINT_DB_get_refresh_order (db_conn, j, &refresh_session_pub, &denom_pub);
     if (GNUNET_OK != res)
     {
       GNUNET_break (0);
-      return GNUNET_SYSERR;
+                    // FIXME: return error code!
+      return MHD_NO;
     }
 
 
@@ -1392,7 +1402,8 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
     if (NULL == dki)
     {
       GNUNET_break (0);
-      return GNUNET_SYSERR;
+                    // FIXME: return error code!
+      return MHD_NO;
     }
     if (GNUNET_OK !=
         TALER_RSA_sign (dki->denom_priv,
@@ -1401,7 +1412,8 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
                         &ev_sig))
     {
       GNUNET_break (0);
-      return GNUNET_SYSERR;
+                    // FIXME: return error code!
+      return MHD_NO;
     }
 
     res = TALER_MINT_DB_insert_refresh_collectable (db_conn,
@@ -1411,7 +1423,8 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
     if (GNUNET_OK != res)
     {
       GNUNET_break (0);
-      return GNUNET_SYSERR;
+                          // FIXME: return error code!
+      return MHD_NO;
     }
   }
   /* mark that reveal was successful */
@@ -1420,7 +1433,8 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh,
   if (GNUNET_OK != res)
   {
     GNUNET_break (0);
-    return GNUNET_SYSERR;
+    // FIXME: return error code!
+    return MHD_NO;
   }
 
   if (GNUNET_OK != TALER_MINT_DB_commit (db_conn))
@@ -1474,7 +1488,8 @@ TALER_MINT_handler_refresh_link (struct RequestHandler *rh,
   if (NULL == (db_conn = TALER_MINT_DB_get_connection ()))
   {
     GNUNET_break (0);
-    return GNUNET_SYSERR;
+    // FIXME: return error code!
+    return MHD_NO;
   }
 
   list = json_array ();
@@ -1488,6 +1503,7 @@ TALER_MINT_handler_refresh_link (struct RequestHandler *rh,
   if (GNUNET_SYSERR == res)
   {
     GNUNET_break (0);
+        // FIXME: return error code!
     return MHD_NO;
   }
   if (GNUNET_NO == res)
@@ -1504,6 +1520,7 @@ TALER_MINT_handler_refresh_link (struct RequestHandler *rh,
   if (GNUNET_SYSERR == res)
   {
     GNUNET_break (0);
+        // FIXME: return error code!
     return MHD_NO;
   }
   if (GNUNET_NO == res)

src/mint/taler-mint-httpd_responses.c

@@ -138,6 +138,71 @@ TALER_MINT_reply_arg_missing (struct MHD_Connection *connection,
 }
 
 
+/**
+ * Send a response indicating an internal error.
+ *
+ * @param connection the MHD connection to use
+ * @param hint hint about the internal error's nature
+ * @return a MHD result code
+ */
+int
+TALER_MINT_reply_internal_error (struct MHD_Connection *connection,
+                                 const char *hint)
+{
+  json_t *json;
+
+  json = json_pack ("{ s:s, s:s }",
+                    "error",
+                    "internal error",
+                    "hint",
+                    hint);
+  return TALER_MINT_reply_json (connection,
+                                json,
+                                MHD_HTTP_BAD_REQUEST);
+}
+
+
+/**
+ * Send a response indicating that the request was too big.
+ *
+ * @param connection the MHD connection to use
+ * @return a MHD result code
+ */
+int
+TALER_MINT_reply_request_too_large (struct MHD_Connection *connection)
+{
+  struct MHD_Response *resp;
+  int ret;
+
+  resp = MHD_create_response_from_buffer (0,
+                                          NULL,
+                                          MHD_RESPMEM_PERSISTENT);
+  if (NULL == resp)
+    return MHD_NO;
+  ret = MHD_queue_response (connection,
+                            MHD_HTTP_REQUEST_ENTITY_TOO_LARGE,
+                            resp);
+  MHD_destroy_response (resp);
+  return ret;
+}
+
+
+/**
+ * Send a response indicating that the JSON was malformed.
+ *
+ * @param connection the MHD connection to use
+ * @return a MHD result code
+ */
+int
+TALER_MINT_reply_invalid_json (struct MHD_Connection *connection)
+{
+  return TALER_MINT_reply_json_pack (connection,
+                                     MHD_HTTP_BAD_REQUEST,
+                                     "{s:s}",
+                                     "error",
+                                     "invalid json");
+}
+
 
 
 

src/mint/taler-mint-httpd_responses.h

@@ -90,6 +90,39 @@ TALER_MINT_reply_arg_missing (struct MHD_Connection *connection,
                               const char *param_name);
 
 
+/**
+ * Send a response indicating an internal error.
+ *
+ * @param connection the MHD connection to use
+ * @param hint hint about the internal error's nature
+ * @return a MHD result code
+ */
+int
+TALER_MINT_reply_internal_error (struct MHD_Connection *connection,
+                                 const char *hint);
+
+
+/**
+ * Send a response indicating that the request was too big.
+ *
+ * @param connection the MHD connection to use
+ * @return a MHD result code
+ */
+int
+TALER_MINT_reply_request_too_large (struct MHD_Connection *connection);
+
+
+/**
+ * Send a response indicating that the JSON was malformed.
+ *
+ * @param connection the MHD connection to use
+ * @return a MHD result code
+ */
+int
+TALER_MINT_reply_invalid_json (struct MHD_Connection *connection);
+
+
+
 
 
 #endif