[settings] Re-add "uristring" setting type

Commit 09b057c ("[settings] Remove "uristring" setting type") removed
support for URI-encoded settings via the "uristring" setting type, on
the basis that such encoding was no longer necessary to avoid problems
with the command line parser.

Other valid use cases for the "uristring" setting type do exist: for
example, a password containing a '/' character expanded via

  chain http://username:${password:uristring}@server.name/boot.php

Restore the existence of the "uristring" setting, avoiding the
potentially large stack allocations that were used in the old code
prior to commit 09b057c ("[settings] Remove "uristring" setting
type").

Requested-by: Robin Smidsrød <robin@smidsrod.no>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

Author: mcb30

src/core/settings.c

@@ -1666,15 +1666,43 @@ const struct setting_type setting_type_string __setting_type = {
 	.format = format_string_setting,
 };
 
-/** A URI-encoded string setting type
+/**
+ * Parse URI-encoded string setting value
  *
- * This setting type is obsolete; the name ":uristring" is retained to
- * avoid breaking existing scripts.
+ * @v type		Setting type
+ * @v value		Formatted setting value
+ * @v buf		Buffer to contain raw value
+ * @v len		Length of buffer
+ * @ret len		Length of raw value, or negative error
  */
+static int parse_uristring_setting ( const struct setting_type *type __unused,
+				     const char *value, void *buf, size_t len ){
+
+	return uri_decode ( value, buf, len );
+}
+
+/**
+ * Format URI-encoded string setting value
+ *
+ * @v type		Setting type
+ * @v raw		Raw setting value
+ * @v raw_len		Length of raw setting value
+ * @v buf		Buffer to contain formatted value
+ * @v len		Length of buffer
+ * @ret len		Length of formatted value, or negative error
+ */
+static int format_uristring_setting ( const struct setting_type *type __unused,
+				      const void *raw, size_t raw_len,
+				      char *buf, size_t len ) {
+
+	return uri_encode ( 0, raw, raw_len, buf, len );
+}
+
+/** A URI-encoded string setting type */
 const struct setting_type setting_type_uristring __setting_type = {
 	.name = "uristring",
-	.parse = parse_string_setting,
-	.format = format_string_setting,
+	.parse = parse_uristring_setting,
+	.format = format_uristring_setting,
 };
 
 /**

src/core/uri.c

@@ -39,34 +39,62 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <ipxe/uri.h>
 
 /**
- * Decode URI field (in place)
+ * Decode URI field
  *
- * @v string		String
+ * @v encoded		Encoded field
+ * @v buf		Data buffer
+ * @v len		Length
+ * @ret len		Length of data
  *
  * URI decoding can never increase the length of a string; we can
  * therefore safely decode in place.
  */
-static void uri_decode ( char *string ) {
-	char *dest = string;
+size_t uri_decode ( const char *encoded, void *buf, size_t len ) {
+	uint8_t *out = buf;
+	unsigned int count = 0;
 	char hexbuf[3];
 	char *hexbuf_end;
 	char c;
 	char decoded;
 	unsigned int skip;
 
 	/* Copy string, decoding escaped characters as necessary */
-	do {
-		c = *(string++);
+	while ( ( c = *(encoded++) ) ) {
 		if ( c == '%' ) {
-			snprintf ( hexbuf, sizeof ( hexbuf ), "%s", string );
+			snprintf ( hexbuf, sizeof ( hexbuf ), "%s", encoded );
 			decoded = strtoul ( hexbuf, &hexbuf_end, 16 );
 			skip = ( hexbuf_end - hexbuf );
-			string += skip;
+			encoded += skip;
 			if ( skip )
 				c = decoded;
 		}
-		*(dest++) = c;
-	} while ( c );
+		if ( count < len )
+			out[count] = c;
+		count++;
+	}
+	return count;
+}
+
+/**
+ * Decode URI field in-place
+ *
+ * @v uri		URI
+ * @v field		URI field index
+ */
+static void uri_decode_inplace ( struct uri *uri, unsigned int field ) {
+	const char *encoded = uri_field ( uri, field );
+	char *decoded = ( ( char * ) encoded );
+	size_t len;
+
+	/* Do nothing if field is not present */
+	if ( ! encoded )
+		return;
+
+	/* Decode field in place */
+	len = uri_decode ( encoded, decoded, strlen ( encoded ) );
+
+	/* Terminate decoded string */
+	decoded[len] = '\0';
 }
 
 /**
@@ -115,10 +143,15 @@ static int uri_character_escaped ( char c, unsigned int field ) {
 	 * '%', the full set of characters with significance to the
 	 * URL parser is "/#:@?".  We choose for each URI field which
 	 * of these require escaping in our use cases.
+	 *
+	 * For the scheme field (equivalently, if field is zero), we
+	 * escape anything that has significance not just for our URI
+	 * parser but for any other URI parsers (e.g. HTTP query
+	 * string parsers, which care about '=' and '&').
 	 */
 	static const char *escaped[URI_FIELDS] = {
-		/* Scheme: escape everything */
-		[URI_SCHEME]	= "/#:@?",
+		/* Scheme or default: escape everything */
+		[URI_SCHEME]	= "/#:@?=&",
 		/* Opaque part: escape characters which would affect
 		 * the reparsing of the URI, allowing everything else
 		 * (e.g. ':', which will appear in iSCSI URIs).
@@ -157,14 +190,16 @@ static int uri_character_escaped ( char c, unsigned int field ) {
 /**
  * Encode URI field
  *
- * @v uri		URI
  * @v field		URI field index
- * @v buf		Buffer to contain encoded string
+ * @v raw		Raw data
+ * @v raw_len		Length of raw data
+ * @v buf		Buffer
  * @v len		Length of buffer
  * @ret len		Length of encoded string (excluding NUL)
  */
-size_t uri_encode ( const char *string, unsigned int field,
+size_t uri_encode ( unsigned int field, const void *raw, size_t raw_len,
 		    char *buf, ssize_t len ) {
+	const uint8_t *raw_bytes = ( ( const uint8_t * ) raw );
 	ssize_t remaining = len;
 	size_t used;
 	char c;
@@ -174,7 +209,8 @@ size_t uri_encode ( const char *string, unsigned int field,
 		buf[0] = '\0';
 
 	/* Copy string, escaping as necessary */
-	while ( ( c = *(string++) ) ) {
+	while ( raw_len-- ) {
+		c = *(raw_bytes++);
 		if ( uri_character_escaped ( c, field ) ) {
 			used = ssnprintf ( buf, remaining, "%%%02X", c );
 		} else {
@@ -187,6 +223,21 @@ size_t uri_encode ( const char *string, unsigned int field,
 	return ( len - remaining );
 }
 
+/**
+ * Encode URI field string
+ *
+ * @v field		URI field index
+ * @v string		String
+ * @v buf		Buffer
+ * @v len		Length of buffer
+ * @ret len		Length of encoded string (excluding NUL)
+ */
+size_t uri_encode_string ( unsigned int field, const char *string,
+			   char *buf, ssize_t len ) {
+
+	return uri_encode ( field, string, strlen ( string ), buf, len );
+}
+
 /**
  * Dump URI for debugging
  *
@@ -368,10 +419,8 @@ struct uri * parse_uri ( const char *uri_string ) {
 	}
 
 	/* Decode fields in-place */
-	for ( field = 0 ; field < URI_FIELDS ; field++ ) {
-		if ( uri_field ( uri, field ) )
-			uri_decode ( ( char * ) uri_field ( uri, field ) );
-	}
+	for ( field = 0 ; field < URI_FIELDS ; field++ )
+		uri_decode_inplace ( uri, field );
 
  done:
 	DBGC ( uri, "URI parsed \"%s\" to", uri_string );
@@ -444,8 +493,8 @@ size_t format_uri ( const struct uri *uri, char *buf, size_t len ) {
 		}
 
 		/* Encode this field */
-		used += uri_encode ( uri_field ( uri, field ), field,
-				     ( buf + used ), ( len - used ) );
+		used += uri_encode_string ( field, uri_field ( uri, field ),
+					    ( buf + used ), ( len - used ) );
 
 		/* Suffix this field, if applicable */
 		if ( ( field == URI_SCHEME ) && ( ! uri->opaque ) ) {

src/include/ipxe/uri.h

@@ -191,8 +191,11 @@ uri_put ( struct uri *uri ) {
 
 extern struct uri *cwuri;
 
-extern size_t uri_encode ( const char *string, unsigned int field,
+extern size_t uri_decode ( const char *encoded, void *buf, size_t len );
+extern size_t uri_encode ( unsigned int field, const void *raw, size_t raw_len,
 			   char *buf, ssize_t len );
+extern size_t uri_encode_string ( unsigned int field, const char *string,
+				  char *buf, ssize_t len );
 extern struct uri * parse_uri ( const char *uri_string );
 extern size_t format_uri ( const struct uri *uri, char *buf, size_t len );
 extern char * format_uri_alloc ( const struct uri *uri );

src/net/tcp/httpcore.c

@@ -1826,7 +1826,7 @@ static size_t http_params ( struct parameters *params, char *buf, size_t len ) {
 		}
 
 		/* URI-encode the key */
-		frag_len = uri_encode ( param->key, 0, buf, remaining );
+		frag_len = uri_encode_string ( 0, param->key, buf, remaining );
 		buf += frag_len;
 		len += frag_len;
 		remaining -= frag_len;
@@ -1839,7 +1839,7 @@ static size_t http_params ( struct parameters *params, char *buf, size_t len ) {
 		remaining--;
 
 		/* URI-encode the value */
-		frag_len = uri_encode ( param->value, 0, buf, remaining );
+		frag_len = uri_encode_string ( 0, param->value, buf, remaining);
 		buf += frag_len;
 		len += frag_len;
 		remaining -= frag_len;

src/tests/settings_test.c

@@ -166,6 +166,12 @@ static struct setting test_string_setting = {
 	.type = &setting_type_string,
 };
 
+/** Test URI-encoded string setting */
+static struct setting test_uristring_setting = {
+	.name = "test_uristring",
+	.type = &setting_type_uristring,
+};
+
 /** Test IPv4 address setting type */
 static struct setting test_ipv4_setting = {
 	.name = "test_ipv4",
@@ -265,6 +271,16 @@ static void settings_test_exec ( void ) {
 	fetchf_ok ( &test_settings, &test_string_setting,
 		    RAW ( 'w', 'o', 'r', 'l', 'd' ), "world" );
 
+	/* "uristring" setting type */
+	storef_ok ( &test_settings, &test_uristring_setting, "hello%20world",
+		    RAW ( 'h', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l',
+			  'd' ) );
+	fetchf_ok ( &test_settings, &test_uristring_setting,
+		    RAW ( 1, 2, 3, 4, 5 ), "%01%02%03%04%05" );
+	fetchf_ok ( &test_settings, &test_uristring_setting,
+		    RAW ( 0, ' ', '%', '/', '#', ':', '@', '?', '=', '&' ),
+		    "%00%20%25%2F%23%3A%40%3F%3D%26" );
+
 	/* "ipv4" setting type */
 	storef_ok ( &test_settings, &test_ipv4_setting, "192.168.0.1",
 		    RAW ( 192, 168, 0, 1 ) );