Fix parsing of URLs with query or fragment delimiters but no path component

Author: triplef

ChangeLog

@@ -1,3 +1,8 @@
+2025-10-22 Frederik Seiffert <frederik@algoriddim.com>
+
+	* Source/NSURL.m: Fix parsing of URLs with query or fragment delimiters
+	but no path component.
+
 2025-10-01 Richard Frith-Macdonald <rfm@gnu.org>
 
 	* Source/Additions/Unicode.m:

Source/NSURL.m

@@ -515,18 +515,18 @@ static id clientForHandle(void *data, NSURLHandle *hdl)
 }
 
 /*
- * Check a string to see if it contains only legal data characters
- * or percent escape sequences.
+ * Check a bounded string (up to end pointer) to see if it contains only
+ * legal data characters or percent escape sequences.
  */
-static BOOL legal(const char *str, const char *extras)
+static BOOL legal_bounded(const char *str, const char *end, const char *extras)
 {
   const char	*mark = "-_.!~*'()";
 
   if (str != 0)
     {
-      while (*str != 0)
+      while (str < end)
 	{
-	  if (*str == '%' && isxdigit(str[1]) && isxdigit(str[2]))
+	  if (*str == '%' && str + 2 < end && isxdigit(str[1]) && isxdigit(str[2]))
 	    {
 	      str += 3;
 	    }
@@ -551,6 +551,17 @@ static BOOL legal(const char *str, const char *extras)
   return YES;
 }
 
+/*
+ * Check a string to see if it contains only legal data characters
+ * or percent escape sequences. Wrapper around legal_bounded.
+ */
+static BOOL legal(const char *str, const char *extras)
+{
+  if (str == 0)
+    return YES;
+  return legal_bounded(str, str + strlen(str), extras);
+}
+
 /*
  * Convert percent escape sequences to individual characters.
  */
@@ -995,41 +1006,78 @@ - (id) initWithString: (NSString*)aUrlString
 	      /*
 	       * Set 'end' to point to the start of the path, or just past
 	       * the 'authority' if there is no path.
+	       * Check for delimiters in order: '/' (path), '?' (query), '#' (fragment).
+	       * We find the authority end without modifying the string, using legal_bounded()
+	       * for validation (RFC 3986).
 	       */
+	      char	*authEnd;	// End of authority section
+	      char	*pathStart = NULL;  // Where path/query/fragment starts
+
 	      end = strchr(start, '/');
 	      if (end == 0)
 		{
 		  buf->hasNoPath = YES;
-		  end = &start[strlen(start)];
+		  char *alt = strchr(start, '?');
+		  if (alt == 0)
+		    {
+		      alt = strchr(start, '#');
+		    }
+		  if (alt == 0)
+		    {
+		      authEnd = &start[strlen(start)];
+		      pathStart = authEnd;
+		    }
+		  else
+		    {
+		      authEnd = alt;
+		      pathStart = alt;
+		    }
 		}
 	      else
 		{
+		  authEnd = end;
+		  pathStart = end + 1;  // Skip the '/' we found
 		  *end++ = '\0';
 		}
 
 	      /*
-	       * Parser username:password part
+	       * Parser username:password part within authority bounds
 	       */
 	      ptr = strchr(start, '@');
-	      if (ptr != 0)
+	      if (ptr != 0 && ptr < authEnd)
 		{
 		  buf->user = start;
+		  char *userEnd = ptr;
 		  *ptr++ = '\0';
 		  start = ptr;
-		  if (legal(buf->user, ";:&=+$,") == NO)
+		  /* Validate user[:password] without the null terminator at ':' */
+		  char *colonPos = strchr(buf->user, ':');
+		  if (colonPos != 0 && colonPos < userEnd)
 		    {
-		      [NSException raise: NSInvalidArgumentException
-                        format: @"[%@ %@](%@, %@) "
-			@"illegal character in user/password part",
-                        NSStringFromClass([self class]),
-                        NSStringFromSelector(_cmd),
-                        aUrlString, aBaseUrl];
+		      if (legal_bounded(buf->user, colonPos, ";:&=+$,") == NO
+			|| legal_bounded(colonPos + 1, userEnd, ";:&=+$,") == NO)
+			{
+			  [NSException raise: NSInvalidArgumentException
+                            format: @"[%@ %@](%@, %@) "
+			    @"illegal character in user/password part",
+                            NSStringFromClass([self class]),
+                            NSStringFromSelector(_cmd),
+                            aUrlString, aBaseUrl];
+			}
+		      *colonPos = '\0';
+		      buf->password = colonPos + 1;
 		    }
-		  ptr = strchr(buf->user, ':');
-		  if (ptr != 0)
+		  else
 		    {
-		      *ptr++ = '\0';
-		      buf->password = ptr;
+		      if (legal_bounded(buf->user, userEnd, ";:&=+$,") == NO)
+			{
+			  [NSException raise: NSInvalidArgumentException
+                            format: @"[%@ %@](%@, %@) "
+			    @"illegal character in user/password part",
+                            NSStringFromClass([self class]),
+                            NSStringFromSelector(_cmd),
+                            aUrlString, aBaseUrl];
+			}
 		    }
 		}
 
@@ -1142,11 +1190,22 @@ - (id) initWithString: (NSString*)aUrlString
 			}
 		    }
 		}
-	      start = end;
+	      start = pathStart;
 	      /* Check for a legal host, unless it's an ipv6 address
 	       * (which would have been checked earlier).
+	       * Use legal_bounded to validate only the host portion without modifying the string.
 	       */
-	      if (*buf->host != '[' && legal(buf->host, "-") == NO)
+	      char *hostEnd = authEnd;
+	      /* Account for port if present */
+	      if (*buf->host != '[')
+		{
+		  char *colon = strchr(buf->host, ':');
+		  if (colon != 0 && colon < authEnd)
+		    {
+		      hostEnd = colon;
+		    }
+		}
+	      if (*buf->host != '[' && legal_bounded(buf->host, hostEnd, "-") == NO)
 		{
 		  [NSException raise: NSInvalidArgumentException
                     format: @"[%@ %@](%@, %@) "

Tests/base/NSURL/basic.m

@@ -392,6 +392,34 @@ int main()
   PASS_EQUAL([rel absoluteString], @"data:,$2A", "relative data URL works");
   PASS_EQUAL([rel baseURL], nil, "Base URL of relative data URL is nil");
 
+  /* Test URLs with query but no path */
+  url = [NSURL URLWithString: @"https://example.com?key=value"];
+  PASS(url != nil, "URL with query but no path should be valid");
+  PASS_EQUAL([url scheme], @"https", "scheme of https://example.com?key=value is https");
+  PASS_EQUAL([url host], @"example.com", "host of https://example.com?key=value is example.com");
+  PASS_EQUAL([url path], @"", "path of https://example.com?key=value is empty");
+  PASS_EQUAL([url query], @"key=value", "query of https://example.com?key=value is key=value");
+  PASS_EQUAL([url absoluteString], @"https://example.com?key=value", "absoluteString works for URL with query but no path");
+
+  /* Test URLs with fragment but no path */
+  url = [NSURL URLWithString: @"https://example.com#section"];
+  PASS(url != nil, "URL with fragment but no path should be valid");
+  PASS_EQUAL([url scheme], @"https", "scheme of https://example.com#section is https");
+  PASS_EQUAL([url host], @"example.com", "host of https://example.com#section is example.com");
+  PASS_EQUAL([url path], @"", "path of https://example.com#section is empty");
+  PASS_EQUAL([url fragment], @"section", "fragment of https://example.com#section is section");
+  PASS_EQUAL([url absoluteString], @"https://example.com#section", "absoluteString works for URL with fragment but no path");
+
+  /* Test URLs with query and fragment but no path */
+  url = [NSURL URLWithString: @"https://example.com?key=value#section"];
+  PASS(url != nil, "URL with query and fragment but no path should be valid");
+  PASS_EQUAL([url scheme], @"https", "scheme of https://example.com?key=value#section is https");
+  PASS_EQUAL([url host], @"example.com", "host of https://example.com?key=value#section is example.com");
+  PASS_EQUAL([url path], @"", "path of https://example.com?key=value#section is empty");
+  PASS_EQUAL([url query], @"key=value", "query of https://example.com?key=value#section is key=value");
+  PASS_EQUAL([url fragment], @"section", "fragment of https://example.com?key=value#section is section");
+  PASS_EQUAL([url absoluteString], @"https://example.com?key=value#section", "absoluteString works for URL with query and fragment but no path");
+
   ///NSURLQueryItem
 
   //OSX behavior is to return query item with an empty string name