uri: form location correctly from relative redirect (#84541)

* uri: form location correctly from relative redirect

Previously, the original URL would be combined with the relative location incorrectly, especially for URL of any complexity.

Add simple tests demonstrating the problem that fail without the fix

* fix pylint error, import the method similar to other uri methods

* add changelog fragment

Signed-off-by: Abhijeet Kasurde <Akasurde@redhat.com>

changelogs/fragments/84540-uri-relative-redirect.yml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - uri - Form location correctly when the server returns a relative redirect (https://github.com/ansible/ansible/issues/84540)

lib/ansible/modules/uri.py

@@ -442,7 +442,7 @@ from datetime import datetime, timezone
 
 from ansible.module_utils.basic import AnsibleModule, sanitize_keys
 from ansible.module_utils.six import binary_type, iteritems, string_types
-from ansible.module_utils.six.moves.urllib.parse import urlencode, urlsplit
+from ansible.module_utils.six.moves.urllib.parse import urlencode, urljoin
 from ansible.module_utils.common.text.converters import to_native, to_text
 from ansible.module_utils.six.moves.collections_abc import Mapping, Sequence
 from ansible.module_utils.urls import (
@@ -505,27 +505,6 @@ def write_file(module, dest, content, resp):
         os.remove(tmpsrc)
 
 
-def absolute_location(url, location):
-    """Attempts to create an absolute URL based on initial URL, and
-    next URL, specifically in the case of a ``Location`` header.
-    """
-
-    if '://' in location:
-        return location
-
-    elif location.startswith('/'):
-        parts = urlsplit(url)
-        base = url.replace(parts[2], '')
-        return '%s%s' % (base, location)
-
-    elif not location.startswith('/'):
-        base = os.path.dirname(url)
-        return '%s/%s' % (base, location)
-
-    else:
-        return location
-
-
 def kv_list(data):
     """ Convert data into a list of key-value tuples """
     if data is None:
@@ -773,7 +752,7 @@ def main():
         uresp[ukey] = value
 
     if 'location' in uresp:
-        uresp['location'] = absolute_location(url, uresp['location'])
+        uresp['location'] = urljoin(url, uresp['location'])
 
     # Default content_encoding to try
     if isinstance(content, binary_type):

test/integration/targets/uri/tasks/redirect-none.yml

@@ -294,3 +294,24 @@
     - http_308_post.redirected == false
     - http_308_post.status == 308
     - http_308_post.url == 'https://' + httpbin_host + '/redirect-to?status_code=308&url=https://' + httpbin_host + '/anything'
+
+- name: Test HTTP return value for location using relative redirects
+  uri:
+    url: https://{{ httpbin_host }}/redirect-to?url={{ item }}
+    status_code: 302
+    follow_redirects: none
+  register: http_302
+  loop:
+    - "/anything?foo=bar"
+    - "status/302"
+    - "./status/302"
+    - "/status/302"
+    - "//{{ httpbin_host }}/status/302"
+    - "https:status/302"
+
+- assert:
+    that:
+      - item.location == ('https://' + httpbin_host + ((idx == 0) | ternary('/anything?foo=bar', '/status/302')))
+  loop: "{{ http_302.results }}"
+  loop_control:
+    index_var: idx

test/integration/targets/uri/tasks/redirect-safe.yml

@@ -272,3 +272,27 @@
     - http_308_post.redirected == false
     - http_308_post.status == 308
     - http_308_post.url == 'https://' + httpbin_host + '/redirect-to?status_code=308&url=https://' + httpbin_host + '/anything'
+
+
+- name: Test HTTP using HEAD with relative path in redirection
+  uri:
+    url: https://{{ httpbin_host }}/redirect-to?status_code={{ item }}&url=/anything?foo=bar
+    follow_redirects: safe
+    return_content: yes
+    method: HEAD
+  register: http_head
+  loop:
+    - '301'
+    - '302'
+    - '303'
+    - '307'
+    - '308'
+
+- assert:
+    that:
+    - item.changed is false
+    - item.json is not defined
+    - item.redirected
+    - item.status == 200
+    - item.url == 'https://' + httpbin_host + '/anything?foo=bar'
+  loop: "{{ http_head.results }}"