URL: return error on malformed URLs with junk after port number

... because it causes confusion with users. Example URLs:

"http://[127.0.0.1]:11211:80" which a lot of languages' URL parsers will
parse and claim uses port number 80, while libcurl would use port number
11211.

"http://user@example.com:80@localhost" which by the WHATWG URL spec will
be treated to contain user name 'user@example.com' but according to
RFC3986 is user name 'user' for the host 'example.com' and then port 80
is followed by "@localhost"

Both these formats are now rejected, and verified so in test 1260.

Reported-by: Orange Tsai

diff --git a/lib/url.c b/lib/url.c
index 2072a61bd61f86a443b9f6711cd655e665d45279..300fc4d14c9b8763562ff51c68fd2f605620a6ae 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -5643,7 +5643,7 @@ static CURLcode parse_remote_port(struct Curl_easy *data,
     }
 #endif
 
-    portptr = strrchr(conn->host.name, ':');
+    portptr = strchr(conn->host.name, ':');
   }
 
   if(data->set.use_port && data->state.allow_port) {
@@ -5698,15 +5698,16 @@ static CURLcode parse_remote_port(struct Curl_easy *data,
       return CURLE_URL_MALFORMAT;
     }
 
-    else if(rest != &portptr[1]) {
+    if(rest[0]) {
+      failf(data, "Port number ended with '%c'", rest[0]);
+      return CURLE_URL_MALFORMAT;
+    }
+
+    if(rest != &portptr[1]) {
       *portptr = '\0'; /* cut off the name there */
       conn->remote_port = curlx_ultous(port);
     }
     else {
-      if(rest[0]) {
-        failf(data, "Illegal port number");
-        return CURLE_URL_MALFORMAT;
-      }
       /* Browser behavior adaptation. If there's a colon with no digits after,
          just cut off the name there which makes us ignore the colon and just
          use the default port. Firefox and Chrome both do that. */

diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index 8251ab9a4d1d059fe7e389021cbbfdc4f717befa..c51cc3595d769af6ce393cd2d391eeea37aa9eee 100644 (file)
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -129,6 +129,7 @@ test1228 test1229 test1230 test1231 test1232 test1233 test1234 test1235 \
 test1236 test1237 test1238 test1239 test1240 test1241 test1242 test1243 \
 test1244 test1245 test1246 test1247 test1248 test1249 test1250 test1251 \
 test1252 test1253 test1254 test1255 test1256 test1257 test1258 test1259 \
+test1260 \
 \
 test1280 test1281 test1282 test1283 test1284 test1285 test1286 \
 \

diff --git a/tests/data/test1260 b/tests/data/test1260
new file mode 100644 (file)
index 0000000..b5a0aef
--- /dev/null
+++ b/tests/data/test1260
@@ -0,0 +1,34 @@
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP GET
+</keywords>
+</info>
+
+# Server-side
+<reply>
+</reply>
+
+# Client-side
+<client>
+<server>
+none
+</server>
+ <name>
+HTTP URL with rubbish after port number
+ </name>
+ <command>
+-g "http://[%HOSTIP]:%HTTPPORT:80/we/want/1260" "http://%HOSTIP:%HTTPPORT:80/we/want/1260" "http://user@example.com:80@localhost"
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+# CURLE_URL_MALFORMAT == 3
+<errorcode>
+3
+</errorcode>
+</protocol>
+</verify>
+</testcase>