[libgovirt] Sanitize append of '/api' to oVirt URI

[Christophe Fergeau <teuf src gnome org>]

commit 086bd8863caf33d3d68c43930cdc9eb74f1e0cc4
Author: Christophe Fergeau <cfergeau redhat com>
Date:   Fri Apr 25 15:17:07 2014 +0200

    Sanitize append of '/api' to oVirt URI
    
    "/api" should not be part of the base oVirt REST URL, but is the
    REST function which is used to access the OvirtApi entry point. The
    current code expects an URI ending in /api, and then strips it from the
    URI in various places.
    This commit cleans this up by making sure the base URI does not include
    "/api" and by adding it in appropriate places. It also automatically
    appends https:// to the give host.

 govirt/ovirt-proxy-deprecated.c |    4 +-
 govirt/ovirt-proxy.c            |   70 +++++++++++++++++++++++++++++----------
 govirt/ovirt-proxy.h            |    2 +-
 govirt/ovirt-resource.c         |    1 -
 govirt/ovirt-rest-call.c        |    8 +---
 govirt/ovirt-utils.c            |   10 -----
 govirt/ovirt-utils.h            |    3 --
 7 files changed, 57 insertions(+), 41 deletions(-)
---
diff --git a/govirt/ovirt-proxy-deprecated.c b/govirt/ovirt-proxy-deprecated.c
index 0859544..fe06703 100644
--- a/govirt/ovirt-proxy-deprecated.c
+++ b/govirt/ovirt-proxy-deprecated.c
@@ -61,7 +61,7 @@ gboolean ovirt_proxy_fetch_vms(OvirtProxy *proxy, GError **error)
 
     g_return_val_if_fail(OVIRT_IS_PROXY(proxy), FALSE);
 
-    vms_node = ovirt_proxy_get_collection_xml(proxy, "vms", error);
+    vms_node = ovirt_proxy_get_collection_xml(proxy, "/api/vms", error);
     if (vms_node == NULL)
         return FALSE;
 
@@ -104,7 +104,7 @@ void ovirt_proxy_fetch_vms_async(OvirtProxy *proxy,
                                          * would trigger a deprecation
                                          * warning */
                                         fetch_vms_async_cb);
-    ovirt_proxy_get_collection_xml_async(proxy, "vms", result, cancellable,
+    ovirt_proxy_get_collection_xml_async(proxy, "/api/vms", result, cancellable,
                                          fetch_vms_async_cb, NULL, NULL);
 }
 
diff --git a/govirt/ovirt-proxy.c b/govirt/ovirt-proxy.c
index 1ec8d31..ea1bf11 100644
--- a/govirt/ovirt-proxy.c
+++ b/govirt/ovirt-proxy.c
@@ -149,7 +149,6 @@ RestXmlNode *ovirt_proxy_get_collection_xml(OvirtProxy *proxy,
     g_return_val_if_fail(OVIRT_IS_PROXY(proxy), NULL);
 
     call = REST_PROXY_CALL(ovirt_action_rest_call_new(REST_PROXY(proxy)));
-    href = ovirt_utils_strip_api_base_dir(href);
     rest_proxy_call_set_function(call, href);
     rest_proxy_call_add_header(call, "All-Content", "true");
 
@@ -247,7 +246,6 @@ OvirtRestCall *ovirt_rest_call_new(OvirtProxy *proxy,
     if (method != NULL) {
         rest_proxy_call_set_method(REST_PROXY_CALL(call), method);
     }
-    href = ovirt_utils_strip_api_base_dir(href);
     rest_proxy_call_set_function(REST_PROXY_CALL(call), href);
     /* FIXME: to set or not to set ?? */
     rest_proxy_call_add_header(REST_PROXY_CALL(call), "All-Content", "true");
@@ -379,19 +377,10 @@ static GFile *get_ca_cert_file(OvirtProxy *proxy)
     gchar *base_uri = NULL;
     gchar *ca_uri = NULL;
     GFile *ca_file = NULL;
-    gsize suffix_len;
 
     g_object_get(G_OBJECT(proxy), "url-format", &base_uri, NULL);
     if (base_uri == NULL)
         goto error;
-    if (g_str_has_suffix(base_uri, OVIRT_API_BASE_DIR))
-        suffix_len = strlen(OVIRT_API_BASE_DIR);
-    else if (g_str_has_suffix(base_uri, "/api"))
-        suffix_len = strlen("/api");
-    else
-        g_return_val_if_reached(NULL);
-
-    base_uri[strlen(base_uri) - suffix_len] = '\0';
 
     ca_uri = g_build_filename(base_uri, CA_CERT_FILENAME, NULL);
     g_debug("CA certificate URI: %s", ca_uri);
@@ -844,15 +833,60 @@ ovirt_proxy_init(OvirtProxy *self)
                                 SOUP_SESSION_FEATURE(self->priv->cookie_jar));
 }
 
-OvirtProxy *ovirt_proxy_new(const char *uri)
+/* FIXME : "uri" should just be a base domain, foo.example.com/some/path
+ * govirt will then prepend https://
+ * /api/ is part of what librest call 'function'
+ * -> get rid of all the /api/ stripping here and there
+ */
+OvirtProxy *ovirt_proxy_new(const char *hostname)
 {
+    char *uri;
+    OvirtProxy *proxy;
+    gsize suffix_len = 0;
+    int i;
+
+    if (!g_str_has_prefix(hostname, "http://";) && !g_str_has_prefix(hostname, "https://";)) {
+        uri = g_strconcat("https://";, hostname, NULL);
+    } else {
+        /* Fallback code for backwards API compat, early libgovirt versions
+         * expected a full fledged URI */
+        g_warning("Passing a full http:// or https:// URI to "
+                  "ovirt_proxy_new() is deprecated");
+        uri = g_strdup(hostname);
+    }
+
+    /* More backwards compat code, strip "/api" from URI */
+    if (g_str_has_suffix(uri, "api")) {
+        suffix_len = strlen("api");
+    } else if (g_str_has_suffix(uri, "/api")) {
+        suffix_len = strlen("/api");
+    } else if (g_str_has_suffix(uri, "/api/")) {
+        suffix_len = strlen("/api/");
+    }
+    if (suffix_len != 0) {
+        g_warning("Passing an URI ending in /api to ovirt_proxy_new() "
+                  "is deprecated");
+        uri[strlen(uri) - suffix_len] = '\0';
+    }
+
+    /* Strip trailing '/' */
+    for (i = strlen(uri)-1; i >= 0; i--) {
+        if (uri[i] != '/') {
+            break;
+        }
+        uri[i] = '\0';
+    }
+
     /* We disable cookies upon OvirtProxy creation as we will be
      * adding our own cookie jar to OvirtProxy
      */
-    return g_object_new(OVIRT_TYPE_PROXY,
-                        "url-format", uri,
-                        "disable-cookies", TRUE,
-                        NULL);
+    proxy =  OVIRT_PROXY(g_object_new(OVIRT_TYPE_PROXY,
+                                      "url-format", uri,
+                                      "disable-cookies", TRUE,
+                                      NULL));
+    g_free(uri);
+
+    return proxy;
 }
 
 
@@ -881,7 +915,7 @@ OvirtApi *ovirt_proxy_fetch_api(OvirtProxy *proxy, GError **error)
 
     g_return_val_if_fail(OVIRT_IS_PROXY(proxy), FALSE);
 
-    api_node = ovirt_proxy_get_collection_xml(proxy, "", error);
+    api_node = ovirt_proxy_get_collection_xml(proxy, "/api", error);
     if (api_node == NULL) {
         return NULL;
     }
@@ -924,7 +958,7 @@ void ovirt_proxy_fetch_api_async(OvirtProxy *proxy,
     result = g_simple_async_result_new (G_OBJECT(proxy), callback,
                                         user_data,
                                         ovirt_proxy_fetch_api_async);
-    ovirt_proxy_get_collection_xml_async(proxy, "", result, cancellable,
+    ovirt_proxy_get_collection_xml_async(proxy, "/api", result, cancellable,
                                          fetch_api_async_cb, NULL, NULL);
 }
 
diff --git a/govirt/ovirt-proxy.h b/govirt/ovirt-proxy.h
index 567c254..d011209 100644
--- a/govirt/ovirt-proxy.h
+++ b/govirt/ovirt-proxy.h
@@ -65,7 +65,7 @@ struct _OvirtProxyClass {
 
 GType ovirt_proxy_get_type(void);
 
-OvirtProxy *ovirt_proxy_new(const char *uri);
+OvirtProxy *ovirt_proxy_new(const char *host);
 
 G_DEPRECATED_FOR(ovirt_collection_lookup_resource)
 OvirtVm *ovirt_proxy_lookup_vm(OvirtProxy *proxy, const char *vm_name);
diff --git a/govirt/ovirt-resource.c b/govirt/ovirt-resource.c
index c813ac7..7cd34fd 100644
--- a/govirt/ovirt-resource.c
+++ b/govirt/ovirt-resource.c
@@ -616,7 +616,6 @@ ovirt_resource_action(OvirtResource *resource, OvirtProxy *proxy,
     g_return_val_if_fail((error == NULL) || (*error == NULL), FALSE);
 
     function = ovirt_resource_get_action(OVIRT_RESOURCE(resource), action);
-    function = ovirt_utils_strip_api_base_dir(function);
     g_return_val_if_fail(function != NULL, FALSE);
 
     call = REST_PROXY_CALL(ovirt_action_rest_call_new(REST_PROXY(proxy)));
diff --git a/govirt/ovirt-rest-call.c b/govirt/ovirt-rest-call.c
index e5ff47d..4095014 100644
--- a/govirt/ovirt-rest-call.c
+++ b/govirt/ovirt-rest-call.c
@@ -90,15 +90,11 @@ static void ovirt_rest_call_set_property(GObject *object,
         rest_proxy_call_set_method(REST_PROXY_CALL(call),
                                    g_value_get_string(value));
         break;
-    case PROP_HREF: {
-        const char *function;
-
+    case PROP_HREF:
         g_free(call->priv->href);
         call->priv->href = g_value_dup_string(value);
-        function = ovirt_utils_strip_api_base_dir(call->priv->href);
-        rest_proxy_call_set_function(REST_PROXY_CALL(call), function);
+        rest_proxy_call_set_function(REST_PROXY_CALL(call), call->priv->href);
         break;
-    }
     default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
     }
diff --git a/govirt/ovirt-utils.c b/govirt/ovirt-utils.c
index b3ea7dc..8fa7312 100644
--- a/govirt/ovirt-utils.c
+++ b/govirt/ovirt-utils.c
@@ -172,16 +172,6 @@ ovirt_utils_guint_from_string(const char *value_str, guint *value)
     return TRUE;
 }
 
-G_GNUC_INTERNAL const char *ovirt_utils_strip_api_base_dir(const char *path)
-{
-    if (g_str_has_prefix(path, OVIRT_API_BASE_DIR)) {
-        g_debug("stripping %s from %s", OVIRT_API_BASE_DIR, path);
-        path += strlen(OVIRT_API_BASE_DIR);
-    }
-
-    return path;
-}
-
 
 G_GNUC_INTERNAL gboolean ovirt_utils_gerror_from_xml_fault(RestXmlNode *root, GError **error)
 {
diff --git a/govirt/ovirt-utils.h b/govirt/ovirt-utils.h
index e92a2ec..0232e26 100644
--- a/govirt/ovirt-utils.h
+++ b/govirt/ovirt-utils.h
@@ -27,8 +27,6 @@
 
 G_BEGIN_DECLS
 
-#define OVIRT_API_BASE_DIR "/api/"
-
 RestXmlNode *ovirt_rest_xml_node_from_call(RestProxyCall *call);
 const char *ovirt_rest_xml_node_get_content(RestXmlNode *node, ...);
 gboolean ovirt_utils_gerror_from_xml_fault(RestXmlNode *root, GError **error);
@@ -43,7 +41,6 @@ int ovirt_utils_genum_get_value (GType enum_type, const char *nick,
 gboolean ovirt_utils_guint64_from_string(const char *value_str, guint64 *value);
 gboolean ovirt_utils_guint_from_string(const char *value_str, guint *value);
 gboolean ovirt_utils_boolean_from_string(const char *value);
-const char *ovirt_utils_strip_api_base_dir(const char *path);
 
 G_END_DECLS