[tftp] Mangle initial slash on TFTP URIs

TFTP URIs are intrinsically problematic, since:

- TFTP servers may use either normal slashes or backslashes as a
  directory separator,

- TFTP servers allow filenames to be specified using relative paths
  (with no initial directory separator),

- TFTP filenames present in a DHCP filename field may use special
  characters such as "?" or "#" that prevent parsing as a generic URI.

As of commit 7667536 ("[uri] Refactor URI parsing and formatting"), we
have directly constructed TFTP URIs from DHCP next-server and filename
pairs, avoiding the generic URI parser.  This eliminated the problems
related to special characters, but indirectly made it impossible to
parse a "tftp://..." URI string into a TFTP URI with a non-absolute
path.

Re-introduce the convention of requiring an extra slash in a
"tftp://..." URI string in order to specify a TFTP URI with an initial
slash in the filename.  For example:

  tftp://192.168.0.1/boot/pxelinux.0  => RRQ "boot/pxelinux.0"
  tftp://192.168.0.1//boot/pxelinux.0 => RRQ "/boot/pxelinux.0"

This is ugly, but there seems to be no other sensible way to provide
the ability to specify all possible TFTP filenames.

A side-effect of this change is that format_uri() will no longer add a
spurious initial "/" when formatting a relative URI string.  This
improves the console output when fetching an image specified via a
relative URI.

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

3 files changed, 60 insertions, 28 deletions

diff --git a/src/core/uri.c b/src/core/uri.c
index 5bb721ff1..a8fdb70a7 100644
--- a/src/core/uri.c
+++ b/src/core/uri.c
@@ -368,7 +368,7 @@ struct uri * parse_uri ( const char *uri_string ) {
 		goto done;
 
 	/* Identify net/absolute/relative path */
-	if ( strncmp ( path, "//", 2 ) == 0 ) {
+	if ( uri->scheme && ( strncmp ( path, "//", 2 ) == 0 ) ) {
 		/* Net path.  If this is terminated by the first '/'
 		 * of an absolute path, then we have no space for a
 		 * terminator after the authority field, so shuffle
@@ -459,7 +459,6 @@ size_t format_uri ( const struct uri *uri, char *buf, size_t len ) {
 		[URI_OPAQUE] = ':',
 		[URI_PASSWORD] = ':',
 		[URI_PORT] = ':',
-		[URI_PATH] = '/',
 		[URI_QUERY] = '?',
 		[URI_FRAGMENT] = '#',
 	};
@@ -486,8 +485,6 @@ size_t format_uri ( const struct uri *uri, char *buf, size_t len ) {
 		prefix = prefixes[field];
 		if ( ( field == URI_HOST ) && ( uri->user != NULL ) )
 			prefix = '@';
-		if ( ( field == URI_PATH ) && ( uri->path[0] == '/' ) )
-			prefix = '\0';
 		if ( prefix ) {
 			used += ssnprintf ( ( buf + used ), ( len - used ),
 					    "%c", prefix );
@@ -715,6 +712,55 @@ struct uri * resolve_uri ( const struct uri *base_uri,
 }
 
 /**
+ * Construct TFTP URI from server address and filename
+ *
+ * @v sa_server		Server address
+ * @v filename		Filename
+ * @ret uri		URI, or NULL on failure
+ */
+static struct uri * tftp_uri ( struct sockaddr *sa_server,
+			       const char *filename ) {
+	struct sockaddr_tcpip *st_server =
+		( ( struct sockaddr_tcpip * ) sa_server );
+	char buf[ 6 /* "65535" + NUL */ ];
+	char *path;
+	struct uri tmp;
+	struct uri *uri = NULL;
+
+	/* Initialise TFTP URI */
+	memset ( &tmp, 0, sizeof ( tmp ) );
+	tmp.scheme = "tftp";
+
+	/* Construct TFTP server address */
+	tmp.host = sock_ntoa ( sa_server );
+	if ( ! tmp.host )
+		goto err_host;
+
+	/* Construct TFTP server port, if applicable */
+	if ( st_server->st_port ) {
+		snprintf ( buf, sizeof ( buf ), "%d",
+			   ntohs ( st_server->st_port ) );
+		tmp.port = buf;
+	}
+
+	/* Construct TFTP path */
+	if ( asprintf ( &path, "/%s", filename ) < 0 )
+		goto err_path;
+	tmp.path = path;
+
+	/* Demangle URI */
+	uri = uri_dup ( &tmp );
+	if ( ! uri )
+		goto err_uri;
+
+ err_uri:
+	free ( path );
+ err_path:
+ err_host:
+	return uri;
+}
+
+/**
  * Construct URI from server address and filename
  *
  * @v sa_server		Server address
@@ -727,10 +773,6 @@ struct uri * resolve_uri ( const struct uri *base_uri,
  * constructing a TFTP URI from the next-server and filename.
  */
 struct uri * pxe_uri ( struct sockaddr *sa_server, const char *filename ) {
-	char buf[ 6 /* "65535" + NUL */ ];
-	struct sockaddr_tcpip *st_server =
-		( ( struct sockaddr_tcpip * ) sa_server );
-	struct uri tmp;
 	struct uri *uri;
 
 	/* Fail if filename is empty */
@@ -748,17 +790,5 @@ struct uri * pxe_uri ( struct sockaddr *sa_server, const char *filename ) {
 	uri_put ( uri );
 
 	/* Otherwise, construct a TFTP URI directly */
-	memset ( &tmp, 0, sizeof ( tmp ) );
-	tmp.scheme = "tftp";
-	tmp.host = sock_ntoa ( sa_server );
-	if ( ! tmp.host )
-		return NULL;
-	if ( st_server->st_port ) {
-		snprintf ( buf, sizeof ( buf ), "%d",
-			   ntohs ( st_server->st_port ) );
-		tmp.port = buf;
-	}
-	tmp.path = filename;
-	uri = uri_dup ( &tmp );
-	return uri;
+	return tftp_uri ( sa_server, filename );
 }
diff --git a/src/net/udp/tftp.c b/src/net/udp/tftp.c
index f3cb34342..4255472ee 100644
--- a/src/net/udp/tftp.c
+++ b/src/net/udp/tftp.c
@@ -325,7 +325,7 @@ void tftp_set_mtftp_port ( unsigned int port ) {
  * @ret rc		Return status code
  */
 static int tftp_send_rrq ( struct tftp_request *tftp ) {
-	const char *path = tftp->uri->path;
+	const char *path = ( tftp->uri->path + 1 /* skip '/' */ );
 	struct tftp_rrq *rrq;
 	size_t len;
 	struct io_buffer *iobuf;
@@ -1067,6 +1067,8 @@ static int tftp_core_open ( struct interface *xfer, struct uri *uri,
 		return -EINVAL;
 	if ( ! uri->path )
 		return -EINVAL;
+	if ( uri->path[0] != '/' )
+		return -EINVAL;
 
 	/* Allocate and populate TFTP structure */
 	tftp = zalloc ( sizeof ( *tftp ) );
diff --git a/src/tests/uri_test.c b/src/tests/uri_test.c
index 42c1c43d4..a068ab33b 100644
--- a/src/tests/uri_test.c
+++ b/src/tests/uri_test.c
@@ -713,9 +713,9 @@ static struct uri_pxe_test uri_pxe_absolute_path = {
 	{
 		.scheme = "tftp",
 		.host = "192.168.0.2",
-		.path = "/absolute/path",
+		.path = "//absolute/path",
 	},
-	"tftp://192.168.0.2/absolute/path",
+	"tftp://192.168.0.2//absolute/path",
 };
 
 /** PXE URI with relative path */
@@ -731,7 +731,7 @@ static struct uri_pxe_test uri_pxe_relative_path = {
 	{
 		.scheme = "tftp",
 		.host = "192.168.0.3",
-		.path = "relative/path",
+		.path = "/relative/path",
 	},
 	"tftp://192.168.0.3/relative/path",
 };
@@ -749,7 +749,7 @@ static struct uri_pxe_test uri_pxe_icky = {
 	{
 		.scheme = "tftp",
 		.host = "10.0.0.6",
-		.path = "C:\\tftpboot\\icky#path",
+		.path = "/C:\\tftpboot\\icky#path",
 	},
 	"tftp://10.0.0.6/C%3A\\tftpboot\\icky%23path",
 };
@@ -769,9 +769,9 @@ static struct uri_pxe_test uri_pxe_port = {
 		.scheme = "tftp",
 		.host = "192.168.0.1",
 		.port = "4069",
-		.path = "/another/path",
+		.path = "//another/path",
 	},
-	"tftp://192.168.0.1:4069/another/path",
+	"tftp://192.168.0.1:4069//another/path",
 };
 
 /** Current working URI test */