extract_uri: fix, test, document

6 files changed, 63 insertions, 44 deletions

diff --git a/src/extract-uri.cpp b/src/extract-uri.cpp
index 228e0c1c9..65401df6e 100644
--- a/src/extract-uri.cpp
+++ b/src/extract-uri.cpp
@@ -16,15 +16,16 @@
 
 // Functions as per 4.3.4 of CSS 2.1
 // http://www.w3.org/TR/CSS21/syndata.html#uri
-gchar *extract_uri( gchar const *s, gchar const** endptr )
+std::string extract_uri(char const *s, char const **endptr)
 {
+    std::string result;
+
     if (!s)
-        return nullptr;
+        return result;
 
-    gchar* result = nullptr;
     gchar const *sb = s;
     if ( strlen(sb) < 4 || strncmp(sb, "url", 3) != 0 ) {
-        return nullptr;
+        return result;
     }
 
     sb += 3;
@@ -54,7 +55,12 @@ gchar *extract_uri( gchar const *s, gchar const** endptr )
             delim = *sb;
             sb++;
         }
-        gchar const* se = sb + 1;
+
+        if (!*sb) {
+            return result;
+        }
+
+        gchar const* se = sb;
         while ( *se && (*se != delim) ) {
             se++;
         }
@@ -67,14 +73,12 @@ gchar *extract_uri( gchar const *s, gchar const** endptr )
                 }
 
                 // back up for any trailing whitespace
-                se--;
-                while ( ( se[-1] == ' ' ) ||
-                        ( se[-1] == '\t' ) )
+                while (se > sb && g_ascii_isspace(se[-1]))
                 {
                     se--;
                 }
 
-                result = g_strndup(sb, se - sb + 1);
+                result = std::string(sb, se);
             } else {
                 gchar const* tail = se + 1;
                 while ( ( *tail == ' ' ) ||
@@ -86,7 +90,7 @@ gchar *extract_uri( gchar const *s, gchar const** endptr )
                     if ( endptr ) {
                         *endptr = tail + 1;
                     }
-                    result = g_strndup(sb, se - sb);
+                    result = std::string(sb, se);
                 }
             }
         }
diff --git a/src/extract-uri.h b/src/extract-uri.h
index a4de3c856..ee773f458 100644
--- a/src/extract-uri.h
+++ b/src/extract-uri.h
@@ -10,7 +10,31 @@
 #ifndef SEEN_EXTRACT_URI_H
 #define SEEN_EXTRACT_URI_H
 
-char *extract_uri(char const *s, char const** endptr = nullptr);
+#include <string>
+
+/**
+ * Parse functional URI notation, as per 4.3.4 of CSS 2.1
+ *
+ * http://www.w3.org/TR/CSS21/syndata.html#uri
+ *
+ * > The format of a URI value is 'url(' followed by optional white space
+ * > followed by an optional single quote (') or double quote (") character
+ * > followed by the URI itself, followed by an optional single quote (')
+ * > or double quote (") character followed by optional white space
+ * > followed by ')'. The two quote characters must be the same.
+ *
+ * Example:
+ * \verbatim
+   url = extract_uri("url('foo')bar", &out);
+   -> url == "foo"
+   -> out == "bar"
+   \endverbatim
+ *
+ * @param s String which starts with "url("
+ * @param[out] endptr points to \c s + N, where N is the number of characters parsed
+ * @return URL string, or empty string on failure
+ */
+std::string extract_uri(char const *s, char const **endptr = nullptr);
 
 
 #endif /* !SEEN_EXTRACT_URI_H */
diff --git a/src/id-clash.cpp b/src/id-clash.cpp
index 2ee358d49..98c693e50 100644
--- a/src/id-clash.cpp
+++ b/src/id-clash.cpp
@@ -107,12 +107,11 @@ find_references(SPObject *elem, refmap_type &refmap)
                 const char *attr = clipboard_properties[i];
                 const gchar *value = sp_repr_css_property(css, attr, nullptr);
                 if (value) {
-                    gchar *uri = extract_uri(value);
-                    if (uri && uri[0] == '#') {
+                    auto uri = extract_uri(value);
+                    if (uri[0] == '#') {
                         IdReference idref = { REF_CLIPBOARD, elem, attr };
-                        refmap[uri+1].push_back(idref);
+                        refmap[uri.c_str() + 1].push_back(idref);
                     }
-                    g_free(uri);
                 }
             }
 
@@ -163,12 +162,11 @@ find_references(SPObject *elem, refmap_type &refmap)
     for (unsigned i = SP_MARKER_LOC_START; i < SP_MARKER_LOC_QTY; i++) {
         const gchar *value = style->marker_ptrs[i]->value;
         if (value) {
-            gchar *uri = extract_uri(value);
-            if (uri && uri[0] == '#') {
+            auto uri = extract_uri(value);
+            if (uri[0] == '#') {
                 IdReference idref = { REF_STYLE, elem, markers[i] };
-                refmap[uri+1].push_back(idref);
+                refmap[uri.c_str() + 1].push_back(idref);
             }
-            g_free(uri);
         }
     }
 
@@ -177,12 +175,11 @@ find_references(SPObject *elem, refmap_type &refmap)
         const char *attr = other_url_properties[i];
         const gchar *value = repr_elem->attribute(attr);
         if (value) {
-            gchar *uri = extract_uri(value);
-            if (uri && uri[0] == '#') {
+            auto uri = extract_uri(value);
+            if (uri[0] == '#') {
                 IdReference idref = { REF_URL, elem, attr };
-                refmap[uri+1].push_back(idref);
+                refmap[uri.c_str() + 1].push_back(idref);
             }
-            g_free(uri);
         }
     }
     
diff --git a/src/object/sp-item.cpp b/src/object/sp-item.cpp
index d6d1bdf56..edf216481 100644
--- a/src/object/sp-item.cpp
+++ b/src/object/sp-item.cpp
@@ -441,15 +441,14 @@ void SPItem::set(SPAttributeEnum key, gchar const* value) {
             break;
         }
         case SP_PROP_CLIP_PATH: {
-            gchar *uri = extract_uri(value);
-            if (uri) {
+            auto uri = extract_uri(value);
+            if (!uri.empty()) {
                 try {
-                    item->clip_ref->attach(Inkscape::URI(uri));
+                    item->clip_ref->attach(Inkscape::URI(uri.c_str()));
                 } catch (Inkscape::BadURIException &e) {
                     g_warning("%s", e.what());
                     item->clip_ref->detach();
                 }
-                g_free(uri);
             } else {
                 item->clip_ref->detach();
             }
@@ -457,15 +456,14 @@ void SPItem::set(SPAttributeEnum key, gchar const* value) {
             break;
         }
         case SP_PROP_MASK: {
-            gchar *uri = extract_uri(value);
-            if (uri) {
+            auto uri = extract_uri(value);
+            if (!uri.empty()) {
                 try {
-                    item->mask_ref->attach(Inkscape::URI(uri));
+                    item->mask_ref->attach(Inkscape::URI(uri.c_str()));
                 } catch (Inkscape::BadURIException &e) {
                     g_warning("%s", e.what());
                     item->mask_ref->detach();
                 }
-                g_free(uri);
             } else {
                 item->mask_ref->detach();
             }
diff --git a/src/object/uri-references.cpp b/src/object/uri-references.cpp
index 3e11cd26c..69ed140e0 100644
--- a/src/object/uri-references.cpp
+++ b/src/object/uri-references.cpp
@@ -243,10 +243,9 @@ SPObject *sp_css_uri_reference_resolve(SPDocument *document, const gchar *uri)
     SPObject *ref = nullptr;
 
     if (document && uri && (strncmp(uri, "url(", 4) == 0)) {
-        gchar *trimmed = extract_uri(uri);
-        if (trimmed) {
-            ref = sp_uri_reference_resolve(document, trimmed);
-            g_free(trimmed);
+        auto trimmed = extract_uri(uri);
+        if (!trimmed.empty()) {
+            ref = sp_uri_reference_resolve(document, trimmed.c_str());
         }
     }
 
diff --git a/src/style-internal.cpp b/src/style-internal.cpp
index b02adec48..37371e18d 100644
--- a/src/style-internal.cpp
+++ b/src/style-internal.cpp
@@ -1245,8 +1245,8 @@ SPIPaint::read( gchar const *str ) {
         if ( strneq(str, "url", 3) ) {
 
             // FIXME: THE FOLLOWING CODE SHOULD BE PUT IN A PRIVATE FUNCTION FOR REUSE
-            gchar *uri = extract_uri( str, &str );
-            if(uri == nullptr || uri[0] == '\0') {
+            auto uri = extract_uri(str, &str);
+            if(uri.empty()) {
                 std::cerr << "SPIPaint::read: url is empty or invalid" << std::endl;
             } else if (!style ) {
                 std::cerr << "SPIPaint::read: url with empty SPStyle pointer" << std::endl;
@@ -1265,11 +1265,9 @@ SPIPaint::read( gchar const *str ) {
                     }
                 }
 
-                // std::cout << "uri: " << (uri?uri:"null") << std::endl;
                 // TODO check what this does in light of move away from union
-                sp_style_set_ipaint_to_uri_string ( style, this, uri);
+                sp_style_set_ipaint_to_uri_string(style, this, uri.c_str());
             }
-            g_free( uri );
         }
 
         while ( g_ascii_isspace(*str) ) {
@@ -1658,8 +1656,8 @@ SPIFilter::read( gchar const *str ) {
     } else if (streq(str, "none")) {
         set = true;
     } else if (strneq(str, "url", 3)) {
-        gchar *uri = extract_uri(str);
-        if (uri == nullptr || uri[0] == '\0') {
+        auto uri = extract_uri(str);
+        if (uri.empty()) {
             std::cerr << "SPIFilter::read: url is empty or invalid" << std::endl;
             return;
         } else if (!style) {
@@ -1684,12 +1682,11 @@ SPIFilter::read( gchar const *str ) {
 
         // We have href
         try {
-            href->attach(Inkscape::URI(uri));
+            href->attach(Inkscape::URI(uri.c_str()));
         } catch (Inkscape::BadURIException &e) {
             std::cerr << "SPIFilter::read() " << e.what() << std::endl;
             href->detach();
         }
-        g_free (uri);
 
     } else {
         std::cerr << "SPIFilter::read(): malformed value: " << str << std::endl;