api: decode query string values only once

`get_string_var()` ran `mg_url_decode()` over the value `mg_get_var()`
returns, but `mg_get_var()` has already decoded it: we leave CivetWeb's
`decode_query_string` at its default `no`, so the query string arrives raw
and CivetWeb decodes it exactly once when extracting a variable.

The second pass changed the value again. `?item=a%2541b` became `aAb`
instead of `a%41b`, and any value containing a space or a `+` made
`mg_url_decode()` return `-1`, so the parameter silently read as absent.
This was harmless enough while `?item=` did not exist, but it is not any
more: we now compare the (once-decoded) path item with the (twice-decoded)
query item and answer 400 when they disagree, so a client naming a group
with a `%` in it got its request rejected.

Return what `mg_get_var()` gives us. Variable *names* are still matched
against the raw query string, so the percent-encoded names in `queries.c`
(e.g. `order%5B0%5D%5Bdir%5D`) keep working.

Adds pytest coverage for both cases: a group whose name contains `%`
addressed through path and `?item=` at once, and one containing a space
addressed through `?item=` alone.

Signed-off-by: DL6ER <dl6er@dl6er.de>

Author: DL6ER

src/webserver/http-common.c

@@ -411,30 +411,12 @@ int get_string_var(const char *source, const char *var, char *dest, size_t dest_
 	if(!source)
 		return -1;
 
-	// Allocate a temporary buffer to store the possibly URI-encoded value
-	// of the variable. We use the real destination later to store the
-	// decoded value. The decoded value will always be shorter than the
-	// encoded value, so using the same length is fine.
-	char *tempbuf = calloc(dest_len, sizeof(char));
-	if(!tempbuf)
-	{
-		log_err("get_string_var: Out of memory");
-		return -1;
-	}
-
-	// Extract value of the particular variable
-	int len = mg_get_var(source, strlen(source), var, tempbuf, dest_len);
-
-	// Decode the URI component if needed
-	if(len > 0)
-		len = mg_url_decode(tempbuf, len, dest, dest_len, 0);
-
-	// Free the temporary buffer, if anything was decoded it's now stored in
-	// dest
-	free(tempbuf);
-
-	// Return the length of the decoded string
-	return len;
+	// Extract the value of the particular variable. mg_get_var() decodes it
+	// for us - the query string reaches us as it was sent because CivetWeb's
+	// decode_query_string is left at its default "no". Decoding a second
+	// time here would corrupt every value containing a literal '%' and let
+	// one containing a space or '+' fail altogether.
+	return mg_get_var(source, strlen(source), var, dest, dest_len);
 }
 
 const char* __attribute__((pure)) startsWith(const char *path, struct ftl_conn *api)

test/api/test_m_mutations.py

@@ -546,6 +546,36 @@ def test_put_group_with_conflicting_item_query_returns_400(self, api_session):
         assert r.status_code == 400, \
             f"Expected 400, got {r.status_code} {r.text}"
 
+    def test_put_and_delete_group_with_percent_in_name(self, api_session):
+        """A '%' survives: path and ?item= are both decoded exactly once."""
+        name = "_pytest_%41_group"
+        url = f"{FTL_URL}/api/groups/{quote(name)}?item={quote(name)}"
+
+        r = api_session.put(url,
+                            json={"comment": "pytest percent", "enabled": True},
+                            timeout=10)
+        assert r.status_code in (200, 201), f"PUT failed: {r.status_code} {r.text}"
+        assert _j(r)["groups"][0]["name"] == name
+
+        # Clean up
+        r = api_session.delete(url, timeout=10)
+        assert r.status_code == 204
+
+    def test_put_and_delete_group_with_space_via_item_query(self, api_session):
+        """A space in ?item= is part of the value, not a decoding error."""
+        name = "_pytest space group"
+        url = f"{FTL_URL}/api/groups?item={quote(name)}"
+
+        r = api_session.put(url,
+                            json={"comment": "pytest space", "enabled": True},
+                            timeout=10)
+        assert r.status_code in (200, 201), f"PUT failed: {r.status_code} {r.text}"
+        assert _j(r)["groups"][0]["name"] == name
+
+        # Clean up
+        r = api_session.delete(url, timeout=10)
+        assert r.status_code == 204
+
 
 # ===========================================================================
 # Batch delete tests