From 66e5351e0adda5891b2ff17ccbafc81f620c0e01 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Sat, 28 Dec 2024 14:47:01 +0100
Subject: [PATCH] urlapi: fix redirect to a new fragment or query (only)
 
The redirect logic was broken when the redirect-to URL was a relative
URL only as a fragment or query (starting with '#' or '?').
 
Extended test 1560 to reproduce, then verify.
 
Reported-by: Jeroen Ooms
Fixes #15836
Closes #15848
 
Conflict:remove doc CURLOPT_PATH_AS_IS.md which is not exist
context adapt
Reference:https://github.com/curl/curl/commit/66e5351e0adda5891b2ff17ccbafc81f620c0e01
---
 lib/urlapi.c            | 103 +++++++++++-----------------------------
 tests/data/test391      |   2 +-
 tests/libtest/lib1560.c |  32 +++++++++++++
 3 files changed, 60 insertions(+), 77 deletions(-)

diff --git a/lib/urlapi.c b/lib/urlapi.c
index 4efab61..f6b0926 100644
--- a/lib/urlapi.c
+++ b/lib/urlapi.c
@@ -256,72 +256,40 @@ static char *concat_url(char *base, const char *relurl)
    problems in the future...
   */
   struct dynbuf newest;
-  char *protsep;
-  char *pathsep;
   bool host_changed = FALSE;
   const char *useurl = relurl;
 
   /* protsep points to the start of the host name */
-  protsep = strstr(base, "//");
+  char *protsep = strstr(base, "//");
+  DEBUGASSERT(protsep);
   if(!protsep)
     protsep = base;
   else
     protsep += 2; /* pass the slashes */
 
-  if('/' != relurl[0]) {
-    int level = 0;
-
-    /* First we need to find out if there's a ?-letter in the URL,
+  if(('/' != relurl[0]) && ('#' != relurl[0])) {
+    /* First we need to find out if there is a ?-letter in the original URL,
        and cut it and the right-side of that off */
-    pathsep = strchr(protsep, '?');
+    char *pathsep = strchr(protsep, '?');
     if(pathsep)
       *pathsep = 0;
 
-    /* we have a relative path to append to the last slash if there's one
-       available, or if the new URL is just a query string (starts with a
-       '?')  we append the new one at the end of the entire currently worked
-       out URL */
-    if(useurl[0] != '?') {
-      pathsep = strrchr(protsep, '/');
+    else {
+      /* if not, cut off the potential fragment */
+      pathsep = strchr(protsep, '#');
       if(pathsep)
         *pathsep = 0;
     }
 
-    /* Check if there's any slash after the host name, and if so, remember
-       that position instead */
-    pathsep = strchr(protsep, '/');
-    if(pathsep)
-      protsep = pathsep + 1;
-    else
-      protsep = NULL;
-
-    /* now deal with one "./" or any amount of "../" in the newurl
-       and act accordingly */
-
-    if((useurl[0] == '.') && (useurl[1] == '/'))
-      useurl += 2; /* just skip the "./" */
-
-    while((useurl[0] == '.') &&
-          (useurl[1] == '.') &&
-          (useurl[2] == '/')) {
-      level++;
-      useurl += 3; /* pass the "../" */
-    }
-
-    if(protsep) {
-      while(level--) {
-        /* cut off one more level from the right of the original URL */
-        pathsep = strrchr(protsep, '/');
-        if(pathsep)
-          *pathsep = 0;
-        else {
-          *protsep = 0;
-          break;
-        }
-      }
+    /* if the redirect-to piece is not just a query, cut the path after the
+       last slash */
+    if(useurl[0] != '?') {
+      pathsep = strrchr(protsep, '/');
+      if(pathsep)
+        pathsep[1] = 0; /* leave the slash */
     }
   }
-  else {
+  else if('/' == relurl[0]) {
     /* We got a new absolute path for this server */
 
     if(relurl[1] == '/') {
@@ -333,29 +301,20 @@ static char *concat_url(char *base, const char *relurl)
       host_changed = TRUE;
     }
     else {
-      /* cut off the original URL from the first slash, or deal with URLs
-         without slash */
-      pathsep = strchr(protsep, '/');
-      if(pathsep) {
-        /* When people use badly formatted URLs, such as
-           "http://www.example.com?dir=/home/daniel" we must not use the first
-           slash, if there's a ?-letter before it! */
-        char *sep = strchr(protsep, '?');
-        if(sep && (sep < pathsep))
-          pathsep = sep;
+      /* cut the original URL at first slash */
+      char *pathsep = strchr(protsep, '/');
+      if(pathsep)
         *pathsep = 0;
-      }
-      else {
-        /* There was no slash. Now, since we might be operating on a badly
-           formatted URL, such as "http://www.example.com?id=2380" which
-           doesn't use a slash separator as it is supposed to, we need to check
-           for a ?-letter as well! */
-        pathsep = strchr(protsep, '?');
-        if(pathsep)
-          *pathsep = 0;
-      }
     }
   }
+  else {
+    /* the relative piece starts with '#' */
+
+    /* If there is a fragment in the original URL, cut it off */
+    char *pathsep = strchr(protsep, '#');
+    if(pathsep)
+      *pathsep = 0;
+  }
 
   Curl_dyn_init(&newest, CURL_MAX_INPUT_LENGTH);
 
@@ -363,14 +322,6 @@ static char *concat_url(char *base, const char *relurl)
   if(Curl_dyn_add(&newest, base))
     return NULL;
 
-  /* check if we need to append a slash */
-  if(('/' == useurl[0]) || (protsep && !*protsep) || ('?' == useurl[0]))
-    ;
-  else {
-    if(Curl_dyn_addn(&newest, "/", 1))
-      return NULL;
-  }
-
   /* then append the new piece on the right side */
   urlencode_str(&newest, useurl, strlen(useurl), !host_changed, FALSE);
 
@@ -1837,7 +1788,7 @@ CURLUcode curl_url_set(CURLU *u, CURLUPart what,
     if(!redired_url)
       return CURLUE_OUT_OF_MEMORY;
 
-    result = parseurl_and_replace(redired_url, u, flags);
+    result = parseurl_and_replace(redired_url, u, flags&~CURLU_PATH_AS_IS);
     free(redired_url);
     return result;
   }
diff --git a/tests/data/test391 b/tests/data/test391
index 24428a0..279c562 100644
--- a/tests/data/test391
+++ b/tests/data/test391
@@ -62,7 +62,7 @@ Host: %HOSTIP:%HTTPPORT
 User-Agent: curl/%VERSION
 Accept: */*
 
-GET /../%TESTNUMBER0002 HTTP/1.1
+GET /%TESTNUMBER0002 HTTP/1.1
 Host: %HOSTIP:%HTTPPORT
 User-Agent: curl/%VERSION
 Accept: */*
diff --git a/tests/libtest/lib1560.c b/tests/libtest/lib1560.c
index 765df0a..d7df5c4 100644
--- a/tests/libtest/lib1560.c
+++ b/tests/libtest/lib1560.c
@@ -1066,6 +1066,38 @@ static CURLUcode updateurl(CURLU *u, const char *cmd, unsigned int setflags)
 }
 
 static const struct redircase set_url_list[] = {
+  {"http://example.org#withs/ash", "/moo#frag",
+   "http://example.org/moo#frag",
+   0, 0, CURLUE_OK},
+  {"http://example.org/", "../path/././../././../moo",
+   "http://example.org/moo",
+   0, 0, CURLUE_OK},
+
+  {"http://example.org?bar/moo", "?weird",
+   "http://example.org/?weird", 0, 0, CURLUE_OK},
+  {"http://example.org/foo?bar", "?weird",
+   "http://example.org/foo?weird", 0, 0, CURLUE_OK},
+  {"http://example.org/foo", "?weird",
+   "http://example.org/foo?weird", 0, 0, CURLUE_OK},
+  {"http://example.org", "?weird",
+   "http://example.org/?weird", 0, 0, CURLUE_OK},
+  {"http://example.org/#original", "?weird#moo",
+   "http://example.org/?weird#moo", 0, 0, CURLUE_OK},
+
+  {"http://example.org?bar/moo#yes/path", "#new/slash",
+   "http://example.org/?bar/moo#new/slash", 0, 0, CURLUE_OK},
+  {"http://example.org/foo?bar", "#weird",
+   "http://example.org/foo?bar#weird", 0, 0, CURLUE_OK},
+  {"http://example.org/foo?bar#original", "#weird",
+   "http://example.org/foo?bar#weird", 0, 0, CURLUE_OK},
+  {"http://example.org/foo#original", "#weird",
+   "http://example.org/foo#weird", 0, 0, CURLUE_OK},
+  {"http://example.org/#original", "#weird",
+   "http://example.org/#weird", 0, 0, CURLUE_OK},
+  {"http://example.org#original", "#weird",
+   "http://example.org/#weird", 0, 0, CURLUE_OK},
+  {"http://example.org/foo?bar", "moo?hey#weird",
+   "http://example.org/moo?hey#weird", 0, 0, CURLUE_OK},
   {"http://example.com/please/../gimme/%TESTNUMBER?foobar#hello",
    "http://example.net/there/it/is/../../tes t case=/%TESTNUMBER0002? yes no",
    "http://example.net/there/tes%20t%20case=/%TESTNUMBER0002?+yes+no",
-- 
2.43.0