-fix cleanup-logic

1 files changed, 155 insertions, 130 deletions

diff --git a/src/backend/taler-merchant-httpd_private-get-orders.c b/src/backend/taler-merchant-httpd_private-get-orders.c
index e3df8309..f2fa788a 100644
--- a/src/backend/taler-merchant-httpd_private-get-orders.c
+++ b/src/backend/taler-merchant-httpd_private-get-orders.c
@@ -24,30 +24,13 @@
 
 
 /**
- * Stores state for adding an order to the array for the response.
+ * Sensible bound on TALER_MERCHANTDB_OrderFilter.delta
  */
-struct AddOrderState
-{
-  /**
-   * The array of orders.
-   */
-  json_t *pa;
-
-  /**
-   * The name of the instance we are querying for.
-   */
-  const char *instance_id;
-
-  /**
-   * The result after adding the orders (#TALER_EC_NONE for okay, anything else for an error).
-   */
-  enum TALER_ErrorCode result;
-
-};
+#define MAX_DELTA 1024
 
 
 /**
- * A pending GET /orders request that is in long polling mode.
+ * A pending GET /orders request.
  */
 struct TMH_PendingOrder
 {
@@ -85,15 +68,29 @@ struct TMH_PendingOrder
   struct GNUNET_TIME_Absolute long_poll_timeout;
 
   /**
-   * State for adding orders. The array `pa` must be
-   * json_decref()'ed when done with the `struct TMH_PendingOrder`!
+   * Filter to apply.
    */
-  struct AddOrderState *aos;
+  struct TALER_MERCHANTDB_OrderFilter of;
 
   /**
-   * Filter to apply.
+   * The array of orders.
    */
-  struct TALER_MERCHANTDB_OrderFilter of;
+  json_t *pa;
+
+  /**
+   * The name of the instance we are querying for.
+   */
+  const char *instance_id;
+
+  /**
+   * The result after adding the orders (#TALER_EC_NONE for okay, anything else for an error).
+   */
+  enum TALER_ErrorCode result;
+
+  /**
+   * Is the structure in the DLL
+   */
+  bool in_dll;
 };
 
 
@@ -121,14 +118,14 @@ TMH_force_get_orders_resume (struct TMH_MerchantInstance *mi)
 
   while (NULL != (po = mi->po_head))
   {
+    GNUNET_assert(po->in_dll);
     GNUNET_CONTAINER_DLL_remove (mi->po_head,
                                  mi->po_tail,
                                  po);
     GNUNET_assert (po ==
                    GNUNET_CONTAINER_heap_remove_root (order_timeout_heap));
     MHD_resume_connection (po->con);
-    json_decref (po->aos->pa);
-    GNUNET_free (po);
+    po->in_dll = false;
   }
   if (NULL != order_timeout_task)
   {
@@ -176,13 +173,13 @@ order_timeout (void *cls)
     GNUNET_log (GNUNET_ERROR_TYPE_INFO,
                 "Resuming long polled job due to timeout\n");
     mi = po->mi;
+    GNUNET_assert(po->in_dll);
     GNUNET_CONTAINER_DLL_remove (mi->po_head,
                                  mi->po_tail,
                                  po);
-    json_decref (po->aos->pa);
+    po->in_dll = false;
     MHD_resume_connection (po->con);
     TMH_trigger_daemon ();   /* we resumed, kick MHD */
-    GNUNET_free (po);
   }
   order_timeout_task = GNUNET_SCHEDULER_add_at (po->long_poll_timeout,
                                                 &order_timeout,
@@ -199,9 +196,21 @@ order_timeout (void *cls)
 static void
 cleanup (void *ctx)
 {
-  struct AddOrderState *aos = ctx;
-  json_decref (aos->pa);
-  GNUNET_free (aos);
+  struct TMH_PendingOrder *po = ctx;
+
+  if (po->in_dll)
+  { 
+    struct TMH_MerchantInstance *mi = po->mi;
+
+    GNUNET_CONTAINER_DLL_remove (mi->po_head,
+                                 mi->po_tail,
+                                 po);
+  }
+  if (NULL != po->hn)
+    GNUNET_assert (po ==
+                   GNUNET_CONTAINER_heap_remove_node (po->hn));
+  json_decref (po->pa);
+  GNUNET_free (po);
 }
 
 
@@ -253,28 +262,26 @@ add_order (void *cls,
            uint64_t order_serial,
            struct GNUNET_TIME_Absolute creation_time)
 {
-  struct AddOrderState *aos = cls;
+  struct TMH_PendingOrder *po = cls;
   json_t *contract_terms;
   struct GNUNET_HashCode h_contract_terms;
   enum GNUNET_DB_QueryStatus qs;
   bool refundable = false;
   bool paid;
 
+  qs = TMH_db->lookup_order_status (TMH_db->cls,
+                                    po->instance_id,
+                                    order_id,
+                                    &h_contract_terms,
+                                    &paid);
+  /* qs == 0: contract terms don't exist, so the order cannot be paid. */
+  if (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs)
+    paid = false;
+  if (qs < 0)
   {
-    qs = TMH_db->lookup_order_status (TMH_db->cls,
-                                      aos->instance_id,
-                                      order_id,
-                                      &h_contract_terms,
-                                      &paid);
-    /* qs == 0: contract terms don't exist, so the order cannot be paid. */
-    if (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs)
-      paid = false;
-    if (qs < 0)
-    {
-      GNUNET_break (0);
-      aos->result = TALER_EC_GENERIC_DB_FETCH_FAILED;
-      return;
-    }
+    GNUNET_break (0);
+    po->result = TALER_EC_GENERIC_DB_FETCH_FAILED;
+    return;
   }
 
   if (paid)
@@ -284,7 +291,7 @@ add_order (void *cls,
     uint64_t os;
 
     qs = TMH_db->lookup_contract_terms (TMH_db->cls,
-                                        aos->instance_id,
+                                        po->instance_id,
                                         order_id,
                                         &contract_terms,
                                         &os);
@@ -294,7 +301,7 @@ add_order (void *cls,
     struct GNUNET_HashCode unused;
 
     qs = TMH_db->lookup_order (TMH_db->cls,
-                               aos->instance_id,
+                               po->instance_id,
                                order_id,
                                NULL,
                                &unused,
@@ -304,7 +311,7 @@ add_order (void *cls,
   if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs)
   {
     GNUNET_break (0);
-    aos->result = TALER_EC_GENERIC_DB_FETCH_FAILED;
+    po->result = TALER_EC_GENERIC_DB_FETCH_FAILED;
     json_decref (contract_terms);
     return;
   }
@@ -312,9 +319,6 @@ add_order (void *cls,
   {
     struct TALER_Amount order_amount;
     struct GNUNET_TIME_Absolute rd;
-
-    struct GNUNET_TIME_Absolute now = GNUNET_TIME_absolute_get ();
-
     struct GNUNET_JSON_Specification spec[] = {
       TALER_JSON_spec_amount ("amount",
                               &order_amount),
@@ -329,12 +333,12 @@ add_order (void *cls,
                            NULL, NULL))
     {
       GNUNET_break (0);
-      aos->result = TALER_EC_MERCHANT_GENERIC_DB_CONTRACT_CONTENT_INVALID;
+      po->result = TALER_EC_MERCHANT_GENERIC_DB_CONTRACT_CONTENT_INVALID;
       json_decref (contract_terms);
       return;
     }
 
-    if ((now.abs_value_us <= rd.abs_value_us) &&
+    if (GNUNET_TIME_absolute_is_future(rd) &&
         paid)
     {
       struct TALER_Amount refund_amount;
@@ -343,14 +347,14 @@ add_order (void *cls,
                      TALER_amount_set_zero (TMH_currency,
                                             &refund_amount));
       qs = TMH_db->lookup_refunds_detailed (TMH_db->cls,
-                                            aos->instance_id,
+                                            po->instance_id,
                                             &h_contract_terms,
                                             &process_refunds_cb,
                                             &refund_amount);
       if (0 > qs)
       {
         GNUNET_break (0);
-        aos->result = TALER_EC_GENERIC_DB_FETCH_FAILED;
+        po->result = TALER_EC_GENERIC_DB_FETCH_FAILED;
         json_decref (contract_terms);
         return;
       }
@@ -362,7 +366,7 @@ add_order (void *cls,
 
   GNUNET_assert (0 ==
                  json_array_append_new (
-                   aos->pa,
+                   po->pa,
                    json_pack (
                      "{s:s, s:I, s:o, s:O, s:O, s:b, s:b}",
                      "order_id",
@@ -436,19 +440,19 @@ TMH_notify_order_change (struct TMH_MerchantInstance *mi,
         continue;
       po->of.delta++;
     }
-    add_order (po->aos,
+    add_order (po,
                order_id,
                order_serial_id,
                date);
+    GNUNET_assert(po->in_dll);
     GNUNET_CONTAINER_DLL_remove (mi->po_head,
                                  mi->po_tail,
                                  po);
+    po->in_dll = false;
     GNUNET_assert (po ==
                    GNUNET_CONTAINER_heap_remove_node (po->hn));
     MHD_resume_connection (po->con);
     TMH_trigger_daemon ();   /* we resumed, kick MHD */
-    json_decref (po->aos->pa);
-    GNUNET_free (po);
   }
 }
 
@@ -466,28 +470,26 @@ TMH_private_get_orders (const struct TMH_RequestHandler *rh,
                         struct MHD_Connection *connection,
                         struct TMH_HandlerContext *hc)
 {
-  struct AddOrderState *aos;
+  struct TMH_PendingOrder *po = hc->ctx;
   enum GNUNET_DB_QueryStatus qs;
   struct TALER_MERCHANTDB_OrderFilter of;
 
-  if (NULL != hc->ctx)
+  if (NULL != po)
   {
     /* resumed from long-polling, return answer we already have
        in 'hc->ctx' */
-    struct AddOrderState *aos = hc->ctx;
-
-    if (TALER_EC_NONE != aos->result)
+    if (TALER_EC_NONE != po->result)
     {
       GNUNET_break (0);
       return TALER_MHD_reply_with_error (connection,
                                          MHD_HTTP_INTERNAL_SERVER_ERROR,
-                                         aos->result,
+                                         po->result,
                                          NULL);
     }
     return TALER_MHD_reply_json_pack (connection,
                                       MHD_HTTP_OK,
                                       "{s:O}",
-                                      "orders", aos->pa);
+                                      "orders", po->pa);
   }
 
   if (! (TALER_arg_to_yna (connection,
@@ -534,11 +536,23 @@ TMH_private_get_orders (const struct TMH_RequestHandler *rh,
                   "%lld%c",
                   &ll,
                   &dummy))
+      {
+	GNUNET_break_op (0);
         return TALER_MHD_reply_with_error (connection,
                                            MHD_HTTP_BAD_REQUEST,
                                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
                                            "delta");
-      of.delta = (uint64_t) ll;
+      }
+      of.delta = (int64_t) ll;
+      if ( (-MAX_DELTA > of.delta) ||
+           (of.delta > MAX_DELTA) )
+      {
+	GNUNET_break_op (0);
+        return TALER_MHD_reply_with_error (connection,
+                                           MHD_HTTP_BAD_REQUEST,
+                                           TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                                           "delta");
+      }
     }
   }
   {
@@ -571,11 +585,10 @@ TMH_private_get_orders (const struct TMH_RequestHandler *rh,
                                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
                                            "date_ms");
       }
-      of.date.abs_value_us = ll * GNUNET_TIME_UNIT_MILLISECONDS.rel_value_us;
-      if (of.date.abs_value_us / GNUNET_TIME_UNIT_MILLISECONDS.rel_value_us !=
-          ll)
+
+      of.date = GNUNET_TIME_absolute_from_ms(ll);
+      if (GNUNET_TIME_absolute_is_never(of.date))
       {
-        /* overflow during multiplication detected */
         GNUNET_break_op (0);
         return TALER_MHD_reply_with_error (connection,
                                            MHD_HTTP_BAD_REQUEST,
@@ -599,19 +612,27 @@ TMH_private_get_orders (const struct TMH_RequestHandler *rh,
     }
     else
     {
-      char dummy[2];
+      char dummy;
       unsigned long long ull;
 
       if (1 !=
           sscanf (start_row_str,
-                  "%llu%1s",
+                  "%llu%c",
                   &ull,
-                  dummy))
+                  &dummy))
         return TALER_MHD_reply_with_error (connection,
                                            MHD_HTTP_BAD_REQUEST,
                                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
                                            "start");
       of.start_row = (uint64_t) ull;
+      if (INT64_MAX < of.start_row)
+      {
+	GNUNET_break_op (0);
+        return TALER_MHD_reply_with_error (connection,
+                                           MHD_HTTP_BAD_REQUEST,
+                                           TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                                           "start");
+      }
     }
   }
   {
@@ -626,69 +647,73 @@ TMH_private_get_orders (const struct TMH_RequestHandler *rh,
     }
     else
     {
-      char dummy[2];
+      char dummy;
       unsigned long long ull;
 
       if (1 !=
           sscanf (timeout_ms_str,
-                  "%lld%1s",
+                  "%lld%c",
                   &ull,
-                  dummy))
+                  &dummy))
         return TALER_MHD_reply_with_error (connection,
                                            MHD_HTTP_BAD_REQUEST,
                                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
                                            "timeout_ms");
       of.timeout = GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_MILLISECONDS,
                                                   ull);
+      if (GNUNET_TIME_relative_is_forever(of.timeout))
+      {
+        GNUNET_break_op (0);
+        return TALER_MHD_reply_with_error (connection,
+                                           MHD_HTTP_BAD_REQUEST,
+                                           TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                                           "timeout_ms");
+      }
     }
-  }
 
-  aos = GNUNET_new (struct AddOrderState);
-  GNUNET_assert (NULL != aos);
-  aos->pa = json_array ();
-  aos->instance_id = hc->instance->settings.id;
-  aos->result = TALER_EC_NONE;
-  GNUNET_assert (NULL != aos->pa);
-  {
-    qs = TMH_db->lookup_orders (TMH_db->cls,
-                                hc->instance->settings.id,
-                                &of,
-                                &add_order,
-                                aos);
-    if (0 > qs)
+    if ( (0 >= of.delta) &&
+         (! GNUNET_TIME_relative_is_zero(of.timeout)) )
     {
-      aos->result = TALER_EC_GENERIC_DB_FETCH_FAILED;
+      GNUNET_break_op (0);
+      of.timeout = GNUNET_TIME_UNIT_ZERO;
     }
-    if (TALER_EC_NONE != aos->result)
-    {
-      enum TALER_ErrorCode aos_result = aos->result;
+  }
 
-      GNUNET_break (0);
-      json_decref (aos->pa);
-      GNUNET_free (aos);
-      return TALER_MHD_reply_with_error (connection,
-                                         MHD_HTTP_INTERNAL_SERVER_ERROR,
-                                         aos_result,
-                                         NULL);
-    }
+  po = GNUNET_new (struct TMH_PendingOrder);
+  hc->ctx = po;
+  hc->cc = &cleanup;
+  po->con = connection;
+  po->pa = json_array ();
+  GNUNET_assert (NULL != po->pa);
+  po->instance_id = hc->instance->settings.id;
+  po->mi = hc->instance;
+
+  qs = TMH_db->lookup_orders (TMH_db->cls,
+                              po->instance_id,
+                              &of,
+                              &add_order,
+                              po);
+  if (0 > qs)
+  {
+    po->result = TALER_EC_GENERIC_DB_FETCH_FAILED;
+  }
+  if (TALER_EC_NONE != po->result)
+  {
+    GNUNET_break (0);
+    return TALER_MHD_reply_with_error (connection,
+                                       MHD_HTTP_INTERNAL_SERVER_ERROR,
+                                       po->result,
+                                       NULL);
   }
-  if ( (0 == qs) &&
-       (of.timeout.rel_value_us > 0) )
+  if ( (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs) &&
+       (! GNUNET_TIME_relative_is_zero(of.timeout)) )
   {
     struct TMH_MerchantInstance *mi = hc->instance;
-    struct TMH_PendingOrder *po;
 
     /* setup timeout heap (if not yet exists) */
     if (NULL == order_timeout_heap)
       order_timeout_heap
         = GNUNET_CONTAINER_heap_create (GNUNET_CONTAINER_HEAP_ORDER_MIN);
-    hc->ctx = aos;
-    hc->cc = &cleanup;
-    po = GNUNET_new (struct TMH_PendingOrder);
-    po->mi = mi;
-    po->con = connection;
-    po->aos = aos;
-    json_incref (po->aos->pa);
     po->hn = GNUNET_CONTAINER_heap_insert (order_timeout_heap,
                                            po,
                                            po->long_poll_timeout.abs_value_us);
@@ -698,23 +723,23 @@ TMH_private_get_orders (const struct TMH_RequestHandler *rh,
                                  mi->po_tail,
                                  po);
     MHD_suspend_connection (connection);
-    /* start timeout task */
-    po = GNUNET_CONTAINER_heap_peek (order_timeout_heap);
-    if (NULL != order_timeout_task)
-      GNUNET_SCHEDULER_cancel (order_timeout_task);
-    order_timeout_task = GNUNET_SCHEDULER_add_at (po->long_poll_timeout,
-                                                  &order_timeout,
-                                                  NULL);
+    {
+      struct TMH_PendingOrder *pot;
+
+      /* start timeout task */
+      pot = GNUNET_CONTAINER_heap_peek (order_timeout_heap);
+      if (NULL != order_timeout_task)
+        GNUNET_SCHEDULER_cancel (order_timeout_task);
+      order_timeout_task = GNUNET_SCHEDULER_add_at (pot->long_poll_timeout,
+                                                    &order_timeout,
+                                                    NULL);
+    }
     return MHD_YES;
   }
-  {
-    json_t *pa = aos->pa;
-    GNUNET_free (aos);
-    return TALER_MHD_reply_json_pack (connection,
-                                      MHD_HTTP_OK,
-                                      "{s:o}",
-                                      "orders", pa);
-  }
+  return TALER_MHD_reply_json_pack (connection,
+                                    MHD_HTTP_OK,
+                                    "{s:O}",
+                                    "orders", po->pa);
 }