[uri] Generalise tftp_uri() to pxe_uri()

Merge the functionality of parse_next_server_and_filename() and
tftp_uri() into a single pxe_uri(), which takes a server address
(IPv4/IPv6/none) and a filename, and produces a URI using the rule:

 - if the filename is a hierarchical absolute URI (i.e. includes a
   scheme such as "http://" or "tftp://") then use that URI and ignore
   the server address,

 - otherwise, if the server address is recognised (according to
   sa_family) then construct a TFTP URI based on the server address,
   port, and filename

 - otherwise fail.

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

src/arch/i386/interface/pxe/pxe_tftp.c

@@ -159,26 +159,21 @@ static struct pxe_tftp_connection pxe_tftp = {
 	.xfer = INTF_INIT ( pxe_tftp_xfer_desc ),
 };
 
-/**
- * Maximum length of a PXE TFTP URI
- *
- * The PXE TFTP API provides 128 characters for the filename; the
- * extra 128 bytes allow for the remainder of the URI.
- */
-#define PXE_TFTP_URI_LEN 256
-
 /**
  * Open PXE TFTP connection
  *
  * @v ipaddress		IP address
- * @v port		TFTP server port
+ * @v port		TFTP server port (in network byte order)
  * @v filename		File name
  * @v blksize		Requested block size
  * @ret rc		Return status code
  */
 static int pxe_tftp_open ( IP4_t ipaddress, UDP_PORT_t port,
 			   UINT8_t *filename, UINT16_t blksize ) {
-	struct in_addr address;
+	union {
+		struct sockaddr sa;
+		struct sockaddr_in sin;
+	} server;
 	struct uri *uri;
 	int rc;
 
@@ -191,12 +186,15 @@ static int pxe_tftp_open ( IP4_t ipaddress, UDP_PORT_t port,
 	pxe_tftp.rc = -EINPROGRESS;
 
 	/* Construct URI */
-	address.s_addr = ipaddress;
-	DBG ( " %s", inet_ntoa ( address ) );
+	memset ( &server, 0, sizeof ( server ) );
+	server.sin.sin_family = AF_INET;
+	server.sin.sin_addr.s_addr = ipaddress;
+	server.sin.sin_port = port;
+	DBG ( " %s", sock_ntoa ( &server.sa ) );
 	if ( port )
 		DBG ( ":%d", ntohs ( port ) );
 	DBG ( ":%s", filename );
-	uri = tftp_uri ( address, ntohs ( port ), ( ( char * ) filename ) );
+	uri = pxe_uri ( &server.sa, ( ( char * ) filename ) );
 	if ( ! uri ) {
 		DBG ( " could not create URI\n" );
 		return -ENOMEM;

src/core/uri.c

@@ -36,6 +36,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <ctype.h>
 #include <ipxe/vsprintf.h>
 #include <ipxe/params.h>
+#include <ipxe/tcpip.h>
 #include <ipxe/uri.h>
 
 /**
@@ -711,30 +712,50 @@ struct uri * resolve_uri ( const struct uri *base_uri,
 }
 
 /**
- * Construct TFTP URI from next-server and filename
+ * Construct URI from server address and filename
  *
- * @v next_server	Next-server address
- * @v port		Port number, or zero to use the default port
+ * @v sa_server		Server address
  * @v filename		Filename
  * @ret uri		URI, or NULL on failure
  *
- * TFTP filenames specified via the DHCP next-server field often
+ * PXE TFTP filenames specified via the DHCP next-server field often
  * contain characters such as ':' or '#' which would confuse the
  * generic URI parser.  We provide a mechanism for directly
  * constructing a TFTP URI from the next-server and filename.
  */
-struct uri * tftp_uri ( struct in_addr next_server, unsigned int port,
-			const char *filename ) {
+struct uri * pxe_uri ( struct sockaddr *sa_server, const char *filename ) {
 	char buf[ 6 /* "65535" + NUL */ ];
-	struct uri uri;
-
-	memset ( &uri, 0, sizeof ( uri ) );
-	uri.scheme = "tftp";
-	uri.host = inet_ntoa ( next_server );
-	if ( port ) {
-		snprintf ( buf, sizeof ( buf ), "%d", port );
-		uri.port = buf;
+	struct sockaddr_tcpip *st_server =
+		( ( struct sockaddr_tcpip * ) sa_server );
+	struct uri tmp;
+	struct uri *uri;
+
+	/* Fail if filename is empty */
+	if ( ! ( filename && filename[0] ) )
+		return NULL;
+
+	/* If filename is a hierarchical absolute URI, then use that
+	 * URI.  (We accept only hierarchical absolute URIs, since PXE
+	 * filenames sometimes start with DOS drive letters such as
+	 * "C:\", which get misinterpreted as opaque absolute URIs.)
+	 */
+	uri = parse_uri ( filename );
+	if ( uri && uri_is_absolute ( uri ) && ( ! uri->opaque ) )
+		return uri;
+	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;
 	}
-	uri.path = filename;
-	return uri_dup ( &uri );
+	tmp.path = filename;
+	uri = uri_dup ( &tmp );
+	return uri;
 }

src/include/ipxe/uri.h

@@ -206,8 +206,8 @@ extern char * resolve_path ( const char *base_path,
 			     const char *relative_path );
 extern struct uri * resolve_uri ( const struct uri *base_uri,
 				  struct uri *relative_uri );
-extern struct uri * tftp_uri ( struct in_addr next_server, unsigned int port,
-			       const char *filename );
+extern struct uri * pxe_uri ( struct sockaddr *sa_server,
+			      const char *filename );
 extern void churi ( struct uri *uri );
 
 #endif /* _IPXE_URI_H */

src/tests/uri_test.c

@@ -35,6 +35,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <string.h>
 #include <byteswap.h>
 #include <ipxe/uri.h>
+#include <ipxe/tcpip.h>
 #include <ipxe/params.h>
 #include <ipxe/test.h>
 
@@ -66,12 +67,15 @@ struct uri_resolve_test {
 	const char *resolved;
 };
 
-/** A TFTP URI test */
-struct uri_tftp_test {
-	/** Next-server address */
-	struct in_addr next_server;
-	/** Port number */
-	unsigned int port;
+/** A PXE URI test */
+struct uri_pxe_test {
+	/** Server address */
+	union {
+		struct sockaddr sa;
+		struct sockaddr_in sin;
+		struct sockaddr_in6 sin6;
+		struct sockaddr_tcpip st;
+	} server;
 	/** Filename */
 	const char *filename;
 	/** URI */
@@ -323,20 +327,20 @@ static void uri_resolve_path_okx ( struct uri_resolve_test *test,
 	uri_resolve_path_okx ( test, __FILE__, __LINE__ )
 
 /**
- * Report URI TFTP test result
+ * Report URI PXE test result
  *
- * @v test		URI TFTP test
+ * @v test		URI PXE test
  * @v file		Test code file
  * @v line		Test code line
  */
-static void uri_tftp_okx ( struct uri_tftp_test *test, const char *file,
-			   unsigned int line ) {
+static void uri_pxe_okx ( struct uri_pxe_test *test, const char *file,
+			  unsigned int line ) {
 	char buf[ strlen ( test->string ) + 1 /* NUL */ ];
 	struct uri *uri;
 	size_t len;
 
 	/* Construct URI */
-	uri = tftp_uri ( test->next_server, test->port, test->filename );
+	uri = pxe_uri ( &test->server.sa, test->filename );
 	okx ( uri != NULL, file, line );
 	if ( uri ) {
 		uri_okx ( uri, &test->uri, file, line );
@@ -346,7 +350,7 @@ static void uri_tftp_okx ( struct uri_tftp_test *test, const char *file,
 	}
 	uri_put ( uri );
 }
-#define uri_tftp_ok( test ) uri_tftp_okx ( test, __FILE__, __LINE__ )
+#define uri_pxe_ok( test ) uri_pxe_okx ( test, __FILE__, __LINE__ )
 
 /**
  * Report current working URI test result
@@ -678,9 +682,33 @@ static struct uri_resolve_test uri_fragment = {
 	"http://192.168.0.254/test#bar",
 };
 
-/** TFTP URI with absolute path */
-static struct uri_tftp_test uri_tftp_absolute = {
-	{ .s_addr = htonl ( 0xc0a80002 ) /* 192.168.0.2 */ }, 0,
+/** PXE URI with absolute URI */
+static struct uri_pxe_test uri_pxe_absolute = {
+	{
+		/* 192.168.0.3 */
+		.sin = {
+			.sin_family = AF_INET,
+			.sin_addr = { .s_addr = htonl ( 0xc0a80003 ) },
+		},
+	},
+	"http://not.a.tftp/uri",
+	{
+		.scheme = "http",
+		.host = "not.a.tftp",
+		.path = "/uri",
+	},
+	"http://not.a.tftp/uri",
+};
+
+/** PXE URI with absolute path */
+static struct uri_pxe_test uri_pxe_absolute_path = {
+	{
+		/* 192.168.0.2 */
+		.sin = {
+			.sin_family = AF_INET,
+			.sin_addr = { .s_addr = htonl ( 0xc0a80002 ) },
+		},
+	},
 	"/absolute/path",
 	{
 		.scheme = "tftp",
@@ -690,9 +718,15 @@ static struct uri_tftp_test uri_tftp_absolute = {
 	"tftp://192.168.0.2/absolute/path",
 };
 
-/** TFTP URI with relative path */
-static struct uri_tftp_test uri_tftp_relative = {
-	{ .s_addr = htonl ( 0xc0a80003 ) /* 192.168.0.3 */ }, 0,
+/** PXE URI with relative path */
+static struct uri_pxe_test uri_pxe_relative_path = {
+	{
+		/* 192.168.0.3 */
+		.sin = {
+			.sin_family = AF_INET,
+			.sin_addr = { .s_addr = htonl ( 0xc0a80003 ) },
+		},
+	},
 	"relative/path",
 	{
 		.scheme = "tftp",
@@ -702,9 +736,15 @@ static struct uri_tftp_test uri_tftp_relative = {
 	"tftp://192.168.0.3/relative/path",
 };
 
-/** TFTP URI with path containing special characters */
-static struct uri_tftp_test uri_tftp_icky = {
-	{ .s_addr = htonl ( 0x0a000006 ) /* 10.0.0.6 */ }, 0,
+/** PXE URI with path containing special characters */
+static struct uri_pxe_test uri_pxe_icky = {
+	{
+		/* 10.0.0.6 */
+		.sin = {
+			.sin_family = AF_INET,
+			.sin_addr = { .s_addr = htonl ( 0x0a000006 ) },
+		},
+	},
 	"C:\\tftpboot\\icky#path",
 	{
 		.scheme = "tftp",
@@ -714,9 +754,16 @@ static struct uri_tftp_test uri_tftp_icky = {
 	"tftp://10.0.0.6/C%3A\\tftpboot\\icky%23path",
 };
 
-/** TFTP URI with custom port */
-static struct uri_tftp_test uri_tftp_port = {
-	{ .s_addr = htonl ( 0xc0a80001 ) /* 192.168.0.1 */ }, 4069,
+/** PXE URI with custom port */
+static struct uri_pxe_test uri_pxe_port = {
+	{
+		/* 192.168.0.1:4069 */
+		.sin = {
+			.sin_family = AF_INET,
+			.sin_addr = { .s_addr = htonl ( 0xc0a80001 ) },
+			.sin_port = htons ( 4069 ),
+		},
+	},
 	"/another/path",
 	{
 		.scheme = "tftp",
@@ -857,11 +904,12 @@ static void uri_test_exec ( void ) {
 	uri_resolve_ok ( &uri_query );
 	uri_resolve_ok ( &uri_fragment );
 
-	/* TFTP URI construction tests */
-	uri_tftp_ok ( &uri_tftp_absolute );
-	uri_tftp_ok ( &uri_tftp_relative );
-	uri_tftp_ok ( &uri_tftp_icky );
-	uri_tftp_ok ( &uri_tftp_port );
+	/* PXE URI construction tests */
+	uri_pxe_ok ( &uri_pxe_absolute );
+	uri_pxe_ok ( &uri_pxe_absolute_path );
+	uri_pxe_ok ( &uri_pxe_relative_path );
+	uri_pxe_ok ( &uri_pxe_icky );
+	uri_pxe_ok ( &uri_pxe_port );
 
 	/* Current working URI tests */
 	uri_churi_ok ( uri_churi );

src/usr/autoboot.c

@@ -87,33 +87,6 @@ __weak int pxe_menu_boot ( struct net_device *netdev __unused ) {
 	return -ENOTSUP;
 }
 
-/**
- * Parse next-server and filename into a URI
- *
- * @v next_server	Next-server address
- * @v filename		Filename
- * @ret uri		URI, or NULL on failure
- */
-static struct uri * parse_next_server_and_filename ( struct in_addr next_server,
-						     const char *filename ) {
-	struct uri *uri;
-
-	/* Parse filename */
-	uri = parse_uri ( filename );
-	if ( ! uri )
-		return NULL;
-
-	/* Construct a TFTP URI for the filename, if applicable */
-	if ( next_server.s_addr && filename[0] && ! uri_is_absolute ( uri ) ) {
-		uri_put ( uri );
-		uri = tftp_uri ( next_server, 0, filename );
-		if ( ! uri )
-			return NULL;
-	}
-
-	return uri;
-}
-
 /** The "keep-san" setting */
 const struct setting keep_san_setting __setting ( SETTING_SANBOOT_EXTRA,
 						  keep-san ) = {
@@ -250,11 +223,17 @@ static void close_all_netdevs ( void ) {
  * @ret uri		URI, or NULL on failure
  */
 struct uri * fetch_next_server_and_filename ( struct settings *settings ) {
-	struct in_addr next_server = { 0 };
+	union {
+		struct sockaddr sa;
+		struct sockaddr_in sin;
+	} next_server;
 	char *raw_filename = NULL;
 	struct uri *uri = NULL;
 	char *filename;
 
+	/* Initialise server address */
+	memset ( &next_server, 0, sizeof ( next_server ) );
+
 	/* If we have a filename, fetch it along with the next-server
 	 * setting from the same settings block.
 	 */
@@ -263,20 +242,27 @@ struct uri * fetch_next_server_and_filename ( struct settings *settings ) {
 		fetch_string_setting_copy ( settings, &filename_setting,
 					    &raw_filename );
 		fetch_ipv4_setting ( settings, &next_server_setting,
-				     &next_server );
+				     &next_server.sin.sin_addr );
+	}
+	if ( ! raw_filename )
+		goto err_fetch;
+
+	/* Populate server address */
+	if ( next_server.sin.sin_addr.s_addr ) {
+		next_server.sin.sin_family = AF_INET;
+		printf ( "Next server: %s\n",
+			 inet_ntoa ( next_server.sin.sin_addr ) );
 	}
 
 	/* Expand filename setting */
-	filename = expand_settings ( raw_filename ? raw_filename : "" );
+	filename = expand_settings ( raw_filename );
 	if ( ! filename )
 		goto err_expand;
-
-	/* Parse next server and filename */
-	if ( next_server.s_addr )
-		printf ( "Next server: %s\n", inet_ntoa ( next_server ) );
 	if ( filename[0] )
 		printf ( "Filename: %s\n", filename );
-	uri = parse_next_server_and_filename ( next_server, filename );
+
+	/* Construct URI */
+	uri = pxe_uri ( &next_server.sa, filename );
 	if ( ! uri )
 		goto err_parse;
 
@@ -284,6 +270,7 @@ struct uri * fetch_next_server_and_filename ( struct settings *settings ) {
 	free ( filename );
  err_expand:
 	free ( raw_filename );
+ err_fetch:
 	return uri;
 }
 
@@ -301,9 +288,11 @@ static struct uri * fetch_root_path ( struct settings *settings ) {
 	/* Fetch root-path setting */
 	fetch_string_setting_copy ( settings, &root_path_setting,
 				    &raw_root_path );
+	if ( ! raw_root_path )
+		goto err_fetch;
 
 	/* Expand filename setting */
-	root_path = expand_settings ( raw_root_path ? raw_root_path : "" );
+	root_path = expand_settings ( raw_root_path );
 	if ( ! root_path )
 		goto err_expand;
 
@@ -318,6 +307,7 @@ static struct uri * fetch_root_path ( struct settings *settings ) {
 	free ( root_path );
  err_expand:
 	free ( raw_root_path );
+ err_fetch:
 	return uri;
 }
 
@@ -378,32 +368,19 @@ int netboot ( struct net_device *netdev ) {
 		goto err_pxe_menu_boot;
 	}
 
-	/* Fetch next server and filename */
+	/* Fetch next server and filename (if any) */
 	filename = fetch_next_server_and_filename ( NULL );
-	if ( ! filename )
-		goto err_filename;
-	if ( ! uri_has_path ( filename ) ) {
-		/* Ignore empty filename */
-		uri_put ( filename );
-		filename = NULL;
-	}
 
-	/* Fetch root path */
+	/* Fetch root path (if any) */
 	root_path = fetch_root_path ( NULL );
-	if ( ! root_path )
-		goto err_root_path;
-	if ( ! uri_is_absolute ( root_path ) ) {
-		/* Ignore empty root path */
-		uri_put ( root_path );
-		root_path = NULL;
-	}
 
 	/* If we have both a filename and a root path, ignore an
-	 * unsupported URI scheme in the root path, since it may
-	 * represent an NFS root.
+	 * unsupported or missing URI scheme in the root path, since
+	 * it may represent an NFS root.
 	 */
 	if ( filename && root_path &&
-	     ( xfer_uri_opener ( root_path->scheme ) == NULL ) ) {
+	     ( ! ( uri_is_absolute ( root_path ) ||
+		   ( xfer_uri_opener ( root_path->scheme ) == NULL ) ) ) ) {
 		printf ( "Ignoring unsupported root path\n" );
 		uri_put ( root_path );
 		root_path = NULL;
@@ -424,9 +401,7 @@ int netboot ( struct net_device *netdev ) {
  err_uriboot:
  err_no_boot:
 	uri_put ( root_path );
- err_root_path:
 	uri_put ( filename );
- err_filename:
  err_pxe_menu_boot:
  err_dhcp:
  err_ifopen: