From 4bd515191b6db342ff465e0595cfeafe6da8a680 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Fri, 16 Jan 2015 14:27:42 +0100 Subject: [PATCH] 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 | 28 ++++--- src/mint/taler-mint-httpd_parsing.c | 53 +++++++----- src/mint/taler-mint-httpd_parsing.h | 5 +- src/mint/taler-mint-httpd_refresh.c | 113 +++++++++++++++----------- src/mint/taler-mint-httpd_responses.c | 65 +++++++++++++++ src/mint/taler-mint-httpd_responses.h | 33 ++++++++ 6 files changed, 215 insertions(+), 82 deletions(-) diff --git a/src/mint/taler-mint-httpd_deposit.c b/src/mint/taler-mint-httpd_deposit.c index f1df0ad89..d92dd5ef7 100644 --- a/src/mint/taler-mint-httpd_deposit.c +++ b/src/mint/taler-mint-httpd_deposit.c @@ -89,21 +89,19 @@ TALER_MINT_handler_deposit (struct RequestHandler *rh, int res; res = TALER_MINT_parse_post_json (connection, - connection_cls, - upload_data, upload_data_size, - &json); + connection_cls, + upload_data, + upload_data_size, + &json); if (GNUNET_SYSERR == res) - { - // FIXME: return 'internal error' - GNUNET_break (0); return MHD_NO; - } - if (GNUNET_NO == res) + if ( (GNUNET_NO == res) || (NULL == json) ) 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); diff --git a/src/mint/taler-mint-httpd_parsing.c b/src/mint/taler-mint-httpd_parsing.c index 9a3fc1d6b..19c88d456 100644 --- a/src/mint/taler-mint-httpd_parsing.c +++ b/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, - MHD_HTTP_BAD_REQUEST, - "{s:s}", - "error", "invalid json")) + TALER_MINT_reply_invalid_json (connection)) ? GNUNET_NO : GNUNET_SYSERR; } *con_cls = NULL; diff --git a/src/mint/taler-mint-httpd_parsing.h b/src/mint/taler-mint-httpd_parsing.h index d9516d486..c725187c5 100644 --- a/src/mint/taler-mint-httpd_parsing.h +++ b/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 diff --git a/src/mint/taler-mint-httpd_refresh.c b/src/mint/taler-mint-httpd_refresh.c index 83225fc15..1f19aedb2 100644 --- a/src/mint/taler-mint-httpd_refresh.c +++ b/src/mint/taler-mint-httpd_refresh.c @@ -607,20 +607,20 @@ TALER_MINT_handler_refresh_melt (struct RequestHandler *rh, struct GNUNET_HashCode melt_hash; res = TALER_MINT_parse_post_json (connection, - connection_cls, - upload_data, - upload_data_size, &root); + connection_cls, + upload_data, + upload_data_size, + &root); if (GNUNET_SYSERR == res) - { - // FIXME: return 'internal error'? - GNUNET_break (0); return MHD_NO; - } - if (GNUNET_NO == res) + if ( (GNUNET_NO == res) || (NULL == root) ) 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, @@ -824,19 +824,15 @@ TALER_MINT_handler_refresh_commit (struct RequestHandler *rh, json_t *root; res = TALER_MINT_parse_post_json (connection, - connection_cls, - upload_data, - upload_data_size, &root); + connection_cls, + upload_data, + upload_data_size, + &root); if (GNUNET_SYSERR == res) - { - // FIXME: return 'internal error'? - GNUNET_break (0); return MHD_NO; - } - if (GNUNET_NO == res) + if ( (GNUNET_NO == res) || (NULL == root) ) return MHD_YES; - res = request_json_require_nav (connection, root, JNAV_FIELD, "session_pub", JNAV_RET_DATA, @@ -1160,16 +1156,13 @@ TALER_MINT_handler_refresh_reveal (struct RequestHandler *rh, json_t *root; res = TALER_MINT_parse_post_json (connection, - connection_cls, - upload_data, upload_data_size, - &root); + connection_cls, + upload_data, + upload_data_size, + &root); if (GNUNET_SYSERR == res) - { - // FIXME: return 'internal error'? - GNUNET_break (0); return MHD_NO; - } - if (GNUNET_NO == res) + if ( (GNUNET_NO == res) || (NULL == root) ) 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)) @@ -1459,9 +1473,9 @@ TALER_MINT_handler_refresh_link (struct RequestHandler *rh, struct SharedSecretEnc shared_secret_enc; res = TALER_MINT_mhd_request_arg_data (connection, - "coin_pub", - &coin_pub, - sizeof (struct GNUNET_CRYPTO_EcdsaPublicKey)); + "coin_pub", + &coin_pub, + sizeof (struct GNUNET_CRYPTO_EcdsaPublicKey)); if (GNUNET_SYSERR == res) { // FIXME: return 'internal error' @@ -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) diff --git a/src/mint/taler-mint-httpd_responses.c b/src/mint/taler-mint-httpd_responses.c index 8f886c3d9..bad4991da 100644 --- a/src/mint/taler-mint-httpd_responses.c +++ b/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"); +} + diff --git a/src/mint/taler-mint-httpd_responses.h b/src/mint/taler-mint-httpd_responses.h index 230904519..be662319f 100644 --- a/src/mint/taler-mint-httpd_responses.h +++ b/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