From ce45aac3a70e196264243606d06905a62eb3adf5 Mon Sep 17 00:00:00 2001 From: Cohen Karnell <7363269+ckarnell@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:42:31 -0400 Subject: [PATCH] Detect an existing URL scheme instead of searching for "http" clean_server_url adds a scheme with `if "http" not in base_url`. That is a substring test, so any host whose NAME contains "http" is treated as already schemed and is returned without one: `myhttpd.local`, `http2.example.com`, `myhttpserver.example.com`. `example.com` correctly becomes `http://example.com`, so the behaviour differs only for hosts that happen to contain those four characters. This is the one thing the hook exists to do. Its docstring is "Fix url scheme" and its class docstring is "fixing common mistakes by users in defining server_url". Match a real scheme instead: RFC 3986 is * followed by "://". Requiring "://" keeps `localhost:8000` unschemed, which is why urlparse is not used for the check, since it reads `localhost` as the scheme. The existing parameterised cases all still pass; none of them used a host containing "http", which is why the suite was green. Added three that do. --- _test_unstructured_client/unit/test_custom_hooks.py | 5 +++++ .../_hooks/custom/clean_server_url_hook.py | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/_test_unstructured_client/unit/test_custom_hooks.py b/_test_unstructured_client/unit/test_custom_hooks.py index cc8e980b..c43279ff 100644 --- a/_test_unstructured_client/unit/test_custom_hooks.py +++ b/_test_unstructured_client/unit/test_custom_hooks.py @@ -204,6 +204,11 @@ def test_unit_clean_server_url_fixes_malformed_paid_api_url(server_url: str): ("localhost:8000", "http://localhost:8000"), ("localhost:8000/general/v0/general", "http://localhost:8000/general/v0/general"), ("http://localhost:8000/general/v0/general", "http://localhost:8000/general/v0/general"), + # -- a host whose NAME contains "http" still needs a scheme added. Testing for a + # -- scheme with `"http" not in base_url` treats these as already schemed. -- + ("myhttpd.local", "http://myhttpd.local"), + ("http2.example.com", "http://http2.example.com"), + ("myhttpserver.example.com/general/v0/general", "http://myhttpserver.example.com/general/v0/general"), ], ) def test_unit_clean_server_url_fixes_non_unst_domain_url(server_url: str, expected_url: str): diff --git a/src/unstructured_client/_hooks/custom/clean_server_url_hook.py b/src/unstructured_client/_hooks/custom/clean_server_url_hook.py index e582df14..77dbc7bc 100644 --- a/src/unstructured_client/_hooks/custom/clean_server_url_hook.py +++ b/src/unstructured_client/_hooks/custom/clean_server_url_hook.py @@ -1,11 +1,18 @@ from __future__ import annotations +import re from typing import Tuple from urllib.parse import ParseResult, urlparse, urlunparse from unstructured_client._hooks.types import SDKInitHook from unstructured_client.httpclient import HttpClient +# A scheme is "* ://" (RFC 3986). Matching on the substring +# "http" instead treats any host containing it, such as `myhttpd.local`, as already +# schemed. Requiring "://" also keeps `localhost:8000` unschemed, which is why urlparse +# is not used for this check: it reads `localhost` as the scheme. +_URL_SCHEME_RE = re.compile(r"^[a-zA-Z][a-zA-Z0-9+.\-]*://") + def clean_server_url(base_url: str | None) -> str: """Fix url scheme and remove subpath for URLs under Unstructured domains.""" @@ -14,7 +21,7 @@ def clean_server_url(base_url: str | None) -> str: return "" # add a url scheme if not present (urllib.parse does not work reliably without it) - if "http" not in base_url: + if not _URL_SCHEME_RE.match(base_url): base_url = "http://" + base_url parsed_url: ParseResult = urlparse(base_url)