[uri] Refactor URI parsing and formatting

Add support for parsing of URIs containing literal IPv6 addresses
(e.g. "http://[fe80::69ff:fe50:5845%25net0]/boot.ipxe").

Duplicate URIs by directly copying the relevant fields, rather than by
formatting and reparsing a URI string.  This relaxes the requirements
on the URI formatting code and allows it to focus on generating
human-readable URIs (e.g. by not escaping ':' characters within
literal IPv6 addresses).  As a side-effect, this allows relative URIs
containing parameter lists (e.g. "../boot.php##params") to function
as expected.

Add validity check for FTP paths to ensure that only printable
characters are accepted (since FTP is a human-readable line-based
protocol with no support for character escaping).

Construct TFTP next-server+filename URIs directly, rather than parsing
a constructed "tftp://..." string,

Add self-tests for URI functions.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

2 files changed, 11 insertions, 23 deletions

diff --git a/src/usr/autoboot.c b/src/usr/autoboot.c
index c95a2566..3a5fb046 100644
--- a/src/usr/autoboot.c
+++ b/src/usr/autoboot.c
@@ -91,8 +91,6 @@ static struct net_device * find_boot_netdev ( void ) {
  */
 static struct uri * parse_next_server_and_filename ( struct in_addr next_server,
 						     const char *filename ) {
-	char buf[ 23 /* "tftp://xxx.xxx.xxx.xxx/" */ + strlen ( filename )
-		  + 1 /* NUL */ ];
 	struct uri *uri;
 
 	/* Parse filename */
@@ -100,17 +98,10 @@ static struct uri * parse_next_server_and_filename ( struct in_addr next_server,
 	if ( ! uri )
 		return NULL;
 
-	/* Construct a tftp:// URI for the filename, if applicable.
-	 * We can't just rely on the current working URI, because the
-	 * relative URI resolution will remove the distinction between
-	 * filenames with and without initial slashes, which is
-	 * significant for TFTP.
-	 */
+	/* Construct a TFTP URI for the filename, if applicable */
 	if ( next_server.s_addr && filename[0] && ! uri_is_absolute ( uri ) ) {
 		uri_put ( uri );
-		snprintf ( buf, sizeof ( buf ), "tftp://%s/%s",
-			   inet_ntoa ( next_server ), filename );
-		uri = parse_uri ( buf );
+		uri = tftp_uri ( next_server, filename );
 		if ( ! uri )
 			return NULL;
 	}
diff --git a/src/usr/imgmgmt.c b/src/usr/imgmgmt.c
index ecf9d31d..1f1f6904 100644
--- a/src/usr/imgmgmt.c
+++ b/src/usr/imgmgmt.c
@@ -44,9 +44,8 @@ FILE_LICENCE ( GPL2_OR_LATER );
  * @ret rc		Return status code
  */
 int imgdownload ( struct uri *uri, struct image **image ) {
-	size_t len = ( unparse_uri ( NULL, 0, uri, URI_ALL ) + 1 );
-	char uri_string_redacted[len];
 	const char *password;
+	char *uri_string_redacted;
 	int rc;
 
 	/* Allocate image */
@@ -56,13 +55,16 @@ int imgdownload ( struct uri *uri, struct image **image ) {
 		goto err_alloc_image;
 	}
 
-	/* Redact password portion of URI, if necessary */
+	/* Construct redacted URI */
 	password = uri->password;
 	if ( password )
 		uri->password = "***";
-	unparse_uri ( uri_string_redacted, sizeof ( uri_string_redacted ),
-		      uri, URI_ALL );
+	uri_string_redacted = format_uri_alloc ( uri );
 	uri->password = password;
+	if ( ! uri_string_redacted ) {
+		rc = -ENOMEM;
+		goto err_uri;
+	}
 
 	/* Create downloader */
 	if ( ( rc = create_downloader ( &monojob, *image, LOCATION_URI,
@@ -81,16 +83,11 @@ int imgdownload ( struct uri *uri, struct image **image ) {
 		goto err_register_image;
 	}
 
-	/* Drop local reference to image.  Image is guaranteed to
-	 * remain in scope since it is registered.
-	 */
-	image_put ( *image );
-
-	return 0;
-
  err_register_image:
  err_monojob_wait:
  err_create_downloader:
+	free ( uri_string_redacted );
+ err_uri:
 	image_put ( *image );
  err_alloc_image:
 	return rc;