[gupnp] ServiceProxy: Properly re-queue if POST failed

[Jens Georg <jensgeorg src gnome org>]

commit 3038b8ecd12ac803c1610d01b3d6a6c2fe06ae3a
Author: Jens Georg <mail jensge org>
Date:   Wed Jan 5 15:25:53 2022 +0100

    ServiceProxy: Properly re-queue if POST failed
    
    The re-queue of message after POST was returned with METHOD_NOT ALLOWED
    never worked. Just re-send the message after changing the method to
    M-POST
    
    Fixes #63

 libgupnp/gupnp-service-proxy.c |  62 ++++++++++++++--
 tests/test-bugs.c              | 161 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 214 insertions(+), 9 deletions(-)
---
diff --git a/libgupnp/gupnp-service-proxy.c b/libgupnp/gupnp-service-proxy.c
index 66ba886..119edf1 100644
--- a/libgupnp/gupnp-service-proxy.c
+++ b/libgupnp/gupnp-service-proxy.c
@@ -633,6 +633,7 @@ on_action_cancelled (GCancellable *cancellable, gpointer user_data)
 static gboolean
 prepare_action_msg (GUPnPServiceProxy *proxy,
                     GUPnPServiceProxyAction *action,
+                    const char *method,
                     GError **error)
 {
         char *control_url, *full_action;
@@ -673,7 +674,7 @@ prepare_action_msg (GUPnPServiceProxy *proxy,
         g_free (control_url);
 
         g_clear_object (&action->msg);
-        action->msg = soup_message_new (SOUP_METHOD_POST, local_control_url);
+        action->msg = soup_message_new (method, local_control_url);
         g_free (local_control_url);
 
         SoupMessageHeaders *headers =
@@ -681,7 +682,21 @@ prepare_action_msg (GUPnPServiceProxy *proxy,
 
         /* Specify action */
         full_action = g_strdup_printf ("\"%s#%s\"", service_type, action->name);
-        soup_message_headers_append (headers, "SOAPAction", full_action);
+
+        // This is internal API, it should either be that constant or the string "M-POST"
+        if (method == SOUP_METHOD_POST) {
+                soup_message_headers_append (headers,
+                                             "SOAPAction",
+                                             full_action);
+        } else {
+                soup_message_headers_append (headers,
+                                             "s-SOAPAction",
+                                             full_action);
+                soup_message_headers_append (
+                        headers,
+                        "Man",
+                        "\"http://schemas.xmlsoap.org/soap/envelope/\";; ns=s");
+        }
         g_free (full_action);
 
         /* Specify language */
@@ -724,6 +739,9 @@ update_message_after_not_allowed (SoupMessage *msg)
         soup_message_headers_remove (headers, "SOAPAction");
 }
 
+static void
+gupnp_service_proxy_action_queue_task (GTask *task);
+
 static void
 action_task_got_response (GObject *source,
                           GAsyncResult *res,
@@ -755,10 +773,40 @@ action_task_got_response (GObject *source,
 
         switch (soup_message_get_status (action->msg)) {
         case SOUP_STATUS_METHOD_NOT_ALLOWED:
-                /* And re-queue */
-                update_message_after_not_allowed (action->msg);
-                // FIXME: soup_session_requeue_message (session, msg);
+                if (g_str_equal (soup_message_get_method (action->msg),
+                                 "POST")) {
+                        g_debug ("POST returned with METHOD_NOT_ALLOWED, "
+                                 "trying with M-POST");
+                        if (!prepare_action_msg (action->proxy,
+                                                 action,
+                                                 "M-POST",
+                                                 &error)) {
+                                if (action->cancellable != NULL && action->cancellable_connection_id != 0) {
+                                        g_cancellable_disconnect (action->cancellable,
+                                                                  action->cancellable_connection_id);
+                                        action->cancellable_connection_id = 0;
+                                }
+
+                                g_task_return_error (task, error);
+
+                                g_object_unref (task);
+                        } else {
+                                gupnp_service_proxy_action_queue_task (task);
+                        }
 
+                } else {
+                        if (action->cancellable != NULL && action->cancellable_connection_id != 0) {
+                                g_cancellable_disconnect (action->cancellable,
+                                                          action->cancellable_connection_id);
+                                action->cancellable_connection_id = 0;
+                        }
+
+                        g_task_return_new_error (task,
+                                                 GUPNP_SERVER_ERROR,
+                                                 GUPNP_SERVER_ERROR_OTHER,
+                                                 "Server does not allow any POST messages");
+                        g_object_unref (task);
+                }
                 break;
 
         default:
@@ -2155,7 +2203,7 @@ gupnp_service_proxy_call_action_async (GUPnPServiceProxy       *proxy,
                               gupnp_service_proxy_action_ref (action),
                               (GDestroyNotify) gupnp_service_proxy_action_unref);
 
-        prepare_action_msg (proxy, action, &error);
+        prepare_action_msg (proxy, action, SOUP_METHOD_POST, &error);
 
         if (error != NULL) {
                 g_task_return_error (task, error);
@@ -2232,7 +2280,7 @@ gupnp_service_proxy_call_action (GUPnPServiceProxy       *proxy,
         g_return_val_if_fail (GUPNP_IS_SERVICE_PROXY (proxy), NULL);
         g_return_val_if_fail (!action->pending, NULL);
 
-        if (!prepare_action_msg (proxy, action, error)) {
+        if (!prepare_action_msg (proxy, action, SOUP_METHOD_POST, error)) {
                 return NULL;
         }
 
diff --git a/tests/test-bugs.c b/tests/test-bugs.c
index 563e758..28ac0e6 100644
--- a/tests/test-bugs.c
+++ b/tests/test-bugs.c
@@ -6,12 +6,14 @@
  * SPDX-License-Identifier: LGPL-2.1-or-later
  */
 
-#include <config.h>
-
 #include <libgupnp/gupnp.h>
 #include <libgupnp/gupnp-service-private.h>
 #include <libgupnp/gupnp-context-private.h>
 
+#include <libgssdp/gssdp-resource-group.h>
+#include <libsoup/soup.h>
+
+
 static GUPnPContext *
 create_context (guint16 port, GError **error) {
         return GUPNP_CONTEXT (g_initable_new (GUPNP_TYPE_CONTEXT,
@@ -686,6 +688,160 @@ test_ggo_42 ()
         g_object_unref (info);
 }
 
+void
+on_control (SoupServer *server,
+            SoupServerMessage *msg,
+            const char *path,
+            GHashTable *query,
+            gpointer user_data)
+{
+        SoupMessageBody *b = soup_server_message_get_response_body (msg);
+
+        g_print ("On Control\n");
+
+        soup_message_body_append (b, SOUP_MEMORY_COPY, "<html>\0", 7);
+        soup_server_message_set_status (msg, SOUP_STATUS_METHOD_NOT_ALLOWED, NULL);
+}
+
+void
+on_event (SoupServer *server,
+          SoupServerMessage *msg,
+          const char *path,
+          GHashTable *query,
+          gpointer user_data)
+{
+        soup_server_message_set_status (msg, SOUP_STATUS_OK, NULL);
+}
+
+typedef struct {
+        GUPnPServiceProxy *p;
+        GMainLoop *loop;
+} TestGGO63Data;
+
+void
+on_proxy (GUPnPControlPoint *cp, GUPnPServiceProxy *p, gpointer user_data)
+{
+        g_print ("Got proxy....\n");
+        TestGGO63Data *d = (TestGGO63Data *) user_data;
+        d->p = g_object_ref (p);
+
+        g_main_loop_quit (d->loop);
+}
+
+void
+on_ping (GObject *object, GAsyncResult *res, gpointer user_data)
+{
+        g_print ("On PIng\n");
+
+        TestGGO63Data *d = (TestGGO63Data *) user_data;
+        GError *error = NULL;
+
+        GUPnPServiceProxyAction *action =
+                gupnp_service_proxy_call_action_finish (
+                        GUPNP_SERVICE_PROXY (object),
+                        res,
+                        &error);
+
+        g_assert_null (action);
+        g_assert_error (error, GUPNP_SERVER_ERROR, GUPNP_SERVER_ERROR_OTHER);
+
+        g_error_free (error);
+
+        g_main_loop_quit (d->loop);
+}
+
+void
+test_ggo_63 ()
+{
+        GUPnPContext *context = NULL;
+        GError *error = NULL;
+
+        context = create_context (0, &error);
+        g_assert_no_error (error);
+        g_assert (context != NULL);
+
+        SoupServer *server = gupnp_context_get_server (context);
+        GSList *uris = soup_server_get_uris (server);
+        GSSDPResourceGroup *rg =
+                gssdp_resource_group_new (GSSDP_CLIENT (context));
+
+        char *server_uri = g_uri_to_string (uris->data);
+        char *resource_path = g_strconcat (server_uri, "TestDevice.xml", NULL);
+        g_free (server_uri);
+        g_slist_free_full (uris, (GDestroyNotify) g_uri_unref);
+
+        gssdp_resource_group_add_resource_simple (rg,
+                                                  "upnp:rootdevice",
+                                                  "uuid:1234::upnp:rootdevice",
+                                                  resource_path);
+
+        gssdp_resource_group_add_resource_simple (rg,
+                                                  "uuid:1234",
+                                                  "uuid:1234",
+                                                  resource_path);
+
+        gssdp_resource_group_add_resource_simple (
+                rg,
+                "urn:test-gupnp-org:device:TestDevice:1",
+                "uuid:1234::urn:test-gupnp-org:device:TestDevice:1",
+                resource_path);
+
+        gssdp_resource_group_add_resource_simple (
+                rg,
+                "urn:test-gupnp-org:service:TestService:1",
+                "uuid:1234::urn:test-gupnp-org:service:TestService:1",
+                resource_path);
+
+        g_free (resource_path);
+
+        gupnp_context_host_path (context,
+                                 DATA_PATH "/TestDevice.xml",
+                                 "/TestDevice.xml");
+
+        soup_server_add_handler (server,
+                                 "/TestService/Control",
+                                 on_control,
+                                 NULL,
+                                 NULL);
+
+        soup_server_add_handler (server,
+                                 "/TestService/Event",
+                                 on_event,
+                                 NULL,
+                                 NULL);
+
+        gssdp_resource_group_set_available (rg, TRUE);
+
+
+        GUPnPControlPoint *cp = gupnp_control_point_new (
+                context,
+                "urn:test-gupnp-org:service:TestService:1");
+
+        TestGGO63Data d = { .loop = g_main_loop_new (NULL, FALSE), .p = NULL };
+        g_signal_connect (G_OBJECT (cp),
+                          "service-proxy-available",
+                          G_CALLBACK (on_proxy),
+                          &d);
+
+        gssdp_resource_browser_set_active (GSSDP_RESOURCE_BROWSER (cp), TRUE);
+
+        g_main_loop_run (d.loop);
+
+        GUPnPServiceProxyAction *a =
+                gupnp_service_proxy_action_new ("Ping", NULL);
+
+        gupnp_service_proxy_call_action_async (d.p, a, NULL, on_ping, &d);
+
+        g_main_loop_run (d.loop);
+        gupnp_service_proxy_action_unref (a);
+
+        g_object_unref (d.p);
+        g_object_unref (cp);
+        g_object_unref (rg);
+        g_object_unref (context);
+        g_main_loop_unref (d.loop);
+}
+
 int
 main (int argc, char *argv[]) {
     g_test_init (&argc, &argv, NULL);
@@ -697,6 +853,7 @@ main (int argc, char *argv[]) {
     g_test_add_func ("/bugs/ggo/24", test_ggo_24);
     g_test_add_func ("/bugs/ggo/58", test_ggo_58);
     g_test_add_func ("/bugs/ggo/42", test_ggo_42);
+    g_test_add_func ("/bugs/ggo/63", test_ggo_63);
 
     return g_test_run ();
 }