URL unescape: return the new length to the caller.

The avoids situations were we threw away the length, only for
the caller to have to strlen the returned string.

Note, there seems to be a case of the amiga front end writing
beyond end of allocation.  Added a TODO for now.

11 files changed, 33 insertions, 17 deletions

diff --git a/content/fetchers/data.c b/content/fetchers/data.c
index 6b8be8dfc..65d99cf14 100644
--- a/content/fetchers/data.c
+++ b/content/fetchers/data.c
@@ -138,7 +138,7 @@ static bool fetch_data_process(struct fetch_data_context *c)
 	char *params;
 	char *comma;
 	char *unescaped;
-	int unescaped_len;
+	size_t unescaped_len;
 	
 	/* format of a data: URL is:
 	 *   data:[<mimetype>][;base64],<data>
@@ -193,14 +193,13 @@ static bool fetch_data_process(struct fetch_data_context *c)
 	/* URL unescape the data first, just incase some insane page
 	 * decides to nest URL and base64 encoding.  Like, say, Acid2.
 	 */
-	res = url_unescape(comma + 1, 0, &unescaped);
+	res = url_unescape(comma + 1, 0, &unescaped_len, &unescaped);
 	if (res != NSERROR_OK) {
 		msg.type = FETCH_ERROR;
 		msg.data.error = "Unable to URL decode data: URL";
 		fetch_data_send_callback(&msg, c);
 		return false;
 	}
-	unescaped_len = strlen(unescaped);
 	
 	if (c->base64) {
 		base64_decode_alloc(unescaped, unescaped_len, &c->data,	&c->datalen);
diff --git a/frontends/amiga/misc.c b/frontends/amiga/misc.c
index 238865120..f15eb48d9 100755
--- a/frontends/amiga/misc.c
+++ b/frontends/amiga/misc.c
@@ -189,6 +189,7 @@ int32 amiga_warn_user_multi(const char *body, const char *opt1, const char *opt2
 static nserror amiga_nsurl_to_path(struct nsurl *url, char **path_out)
 {
 	lwc_string *urlpath;
+	size_t path_len;
 	char *path;
 	bool match;
 	lwc_string *scheme;
@@ -217,7 +218,7 @@ static nserror amiga_nsurl_to_path(struct nsurl *url, char **path_out)
 		return NSERROR_BAD_PARAMETER;
 	}
 
-	res = url_unescape(lwc_string_data(urlpath) + 1, 0, &path);
+	res = url_unescape(lwc_string_data(urlpath) + 1, 0, &path_len, &path);
 	lwc_string_unref(urlpath);
 	if (res != NSERROR_OK) {
 		return res;
@@ -233,9 +234,10 @@ static nserror amiga_nsurl_to_path(struct nsurl *url, char **path_out)
 		}
 		else
 		{
-			int len = strlen(path);
-			path[len] = ':';
-			path[len + 1] = '\0';
+			path[path_len] = ':';
+			/* TODO: Looks like we are writing past the end of
+			 * path's allocation here. */
+			path[path_len + 1] = '\0';
 		}
 	}
 
diff --git a/frontends/atari/file.c b/frontends/atari/file.c
index 7bc11dabc..3816e476d 100644
--- a/frontends/atari/file.c
+++ b/frontends/atari/file.c
@@ -112,6 +112,7 @@ static nserror atari_basename(const char *path, char **str, size_t *size)
 static nserror atari_nsurl_to_path(struct nsurl *url, char **path_out)
 {
 	lwc_string *urlpath;
+	size_t path_len;
 	char *path;
 	bool match;
 	lwc_string *scheme;
@@ -140,6 +141,7 @@ static nserror atari_nsurl_to_path(struct nsurl *url, char **path_out)
 
 	res = url_unescape(lwc_string_data(urlpath),
 			   lwc_string_length(urlpath),
+			   &path_len,
 			   &path);
 	lwc_string_unref(urlpath);
 	if (res != NSERROR_OK) {
@@ -153,7 +155,7 @@ static nserror atari_nsurl_to_path(struct nsurl *url, char **path_out)
 		 * strlen is *not* copying too much data as we are
 		 * moving the null too!
 		 */
-		memmove(path, path + 1, strlen(path));
+		memmove(path, path + 1, path_len);
 	}
 	/* if the path does not have a drive letter we return the
 	 * complete path.
diff --git a/frontends/beos/gui.cpp b/frontends/beos/gui.cpp
index 53387ad07..8c6614d17 100644
--- a/frontends/beos/gui.cpp
+++ b/frontends/beos/gui.cpp
@@ -797,7 +797,7 @@ static char *url_to_path(const char *url)
 	char *url_path;
 	char *path = NULL;
 
-	if (url_unescape(url, 0, &url_path) == NSERROR_OK) {
+	if (url_unescape(url, 0, NULL, &url_path) == NSERROR_OK) {
 		/* return the absolute path including leading / */
 		path = strdup(url_path + (FILE_SCHEME_PREFIX_LEN - 1));
 		free(url_path);
diff --git a/frontends/riscos/download.c b/frontends/riscos/download.c
index 1a0249e20..561409ed1 100644
--- a/frontends/riscos/download.c
+++ b/frontends/riscos/download.c
@@ -241,6 +241,7 @@ static nserror download_ro_filetype(download_context *ctx, bits *ftype_out)
 				char *raw_path;
 				if (url_unescape(lwc_string_data(path),
 						 lwc_string_length(path),
+						 NULL,
 						 &raw_path) == NSERROR_OK) {
 					ftype =	ro_filetype_from_unix_path(raw_path);
 					free(raw_path);
diff --git a/frontends/riscos/gui.c b/frontends/riscos/gui.c
index f55392f99..72eeb6282 100644
--- a/frontends/riscos/gui.c
+++ b/frontends/riscos/gui.c
@@ -1485,6 +1485,7 @@ static nserror ro_path_to_nsurl(const char *path, struct nsurl **url_out)
 static nserror ro_nsurl_to_path(struct nsurl *url, char **path_out)
 {
 	lwc_string *urlpath;
+	size_t unpath_len;
 	char *unpath;
 	char *path;
 	bool match;
@@ -1515,6 +1516,7 @@ static nserror ro_nsurl_to_path(struct nsurl *url, char **path_out)
 
 	res = url_unescape(lwc_string_data(urlpath),
 			   lwc_string_length(urlpath),
+			   &unpath_len,
 			   &unpath);
 	lwc_string_unref(urlpath);
 	if (res != NSERROR_OK) {
@@ -1522,14 +1524,14 @@ static nserror ro_nsurl_to_path(struct nsurl *url, char **path_out)
 	}
 
 	/* RISC OS path should not be more than 100 characters longer */
-	path = malloc(strlen(unpath) + 100);
+	path = malloc(unpath_len + 100);
 	if (path == NULL) {
 		free(unpath);
 		return NSERROR_NOMEM;
 	}
 
 	r = __riscosify(unpath, 0, __RISCOSIFY_NO_SUFFIX,
-			path, strlen(unpath) + 100, 0);
+			path, unpath_len + 100, 0);
 	free(unpath);
 	if (r == NULL) {
 		free(path);
diff --git a/frontends/windows/file.c b/frontends/windows/file.c
index 7583790e9..90e6ef458 100644
--- a/frontends/windows/file.c
+++ b/frontends/windows/file.c
@@ -143,6 +143,7 @@ static nserror windows_nsurl_to_path(struct nsurl *url, char **path_out)
 
 	res = url_unescape(lwc_string_data(urlpath),
 			   lwc_string_length(urlpath),
+			   NULL,
 			   &path);
 	lwc_string_unref(urlpath);
 	if (res != NSERROR_OK) {
diff --git a/test/llcache.c b/test/llcache.c
index df51386a5..8aae357b0 100644
--- a/test/llcache.c
+++ b/test/llcache.c
@@ -95,7 +95,7 @@ char *url_to_path(const char *url)
 	char *url_path;
 	char *path = NULL;
 
-	if (url_unescape(url, 0, &url_path) == NSERROR_OK) {
+	if (url_unescape(url, 0, NULL, &url_path) == NSERROR_OK) {
 		/* return the absolute path including leading / */
 		path = strdup(url_path + (FILE_SCHEME_PREFIX_LEN - 1));
 		free(url_path);
diff --git a/utils/file.c b/utils/file.c
index 6224d1c3c..3ec97dec6 100644
--- a/utils/file.c
+++ b/utils/file.c
@@ -138,6 +138,7 @@ static nserror posix_nsurl_to_path(struct nsurl *url, char **path_out)
 
 	res = url_unescape(lwc_string_data(urlpath),
 			   lwc_string_length(urlpath),
+			   NULL,
 			   &path);
 	lwc_string_unref(urlpath);
 	if (res != NSERROR_OK) {
diff --git a/utils/url.c b/utils/url.c
index 3be983d78..9294e3d31 100644
--- a/utils/url.c
+++ b/utils/url.c
@@ -54,7 +54,8 @@ static inline char xdigit_to_hex(char c)
 
 
 /* exported interface documented in utils/url.h */
-nserror url_unescape(const char *str, size_t length, char **result_out)
+nserror url_unescape(const char *str, size_t length,
+		size_t *length_out, char **result_out)
 {
 	const char *str_end;
 	size_t new_len;
@@ -106,6 +107,9 @@ nserror url_unescape(const char *str, size_t length, char **result_out)
 		}
 	}
 
+	if (length_out != NULL) {
+		*length_out = new_len;
+	}
 	*result_out = result;
 	return NSERROR_OK;
 }
diff --git a/utils/url.h b/utils/url.h
index e67d69a4f..07ad1a794 100644
--- a/utils/url.h
+++ b/utils/url.h
@@ -45,11 +45,15 @@ nserror url_escape(const char *unescaped, size_t toskip, bool sptoplus,
 /**
  * Convert an escaped string to plain.
  *
- * \param[in] str String to unescape.
- * \param[in] length Length of string or 0 to use strlen
- * \param[out] result unescaped string owned by caller must be freed with free()
+ * \param[in]  str         String to unescape.
+ * \param[in]  length      Length of string or 0 to use strlen.
+ * \param[out] length_out  Iff non-NULL, value updated to length of returned
+ *                         result_out string.
+ * \param[out] result_out  Returns unescaped string, owned by caller.
+ *                         Must be freed with free().
  * \return NSERROR_OK on success
  */
-nserror url_unescape(const char *str, size_t length, char **result);
+nserror url_unescape(const char *str, size_t length,
+		size_t *length_out, char **result_out);
 
 #endif