Skip to content

Commit 0094be0

Browse files
committed
Bypass proxy lookups in requests backend for Unix domain hostname paths
Addresses issue found by CI for hostname labels >63 characters. Also the tests have been adapted to always use a Unix domain socket path of 96 bytes length, which is the maximum portably supported length.
1 parent a68f8d7 commit 0094be0

File tree

2 files changed

+47
-4
lines changed

2 files changed

+47
-4
lines changed

ipfshttpclient/http_requests.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,10 @@ def map_args_to_requests(
7171

7272

7373
class ClientSync(ClientSyncBase[requests.Session]): # type: ignore[name-defined]
74-
__slots__ = ("_base_url", "_default_timeout", "_session_props")
74+
__slots__ = ("_base_url", "_default_timeout", "_request_proxies", "_session_props")
7575
#_base_url: str
7676
#_default_timeout: timeout_t
77+
#_request_proxies: ty.Optional[ty.Dict[str, str]]
7778
#_session_props: ty.Dict[str, ty.Any]
7879

7980
def _init(self, addr: addr_t, base: str, *, # type: ignore[no-any-unimported]
@@ -82,7 +83,7 @@ def _init(self, addr: addr_t, base: str, *, # type: ignore[no-any-unimported]
8283
headers: headers_t,
8384
params: params_t,
8485
timeout: timeout_t) -> None:
85-
self._base_url, _, family, host_numeric = multiaddr_to_url_data(addr, base)
86+
self._base_url, uds_path, family, host_numeric = multiaddr_to_url_data(addr, base)
8687

8788
self._session_props = map_args_to_requests(
8889
auth=auth,
@@ -93,6 +94,17 @@ def _init(self, addr: addr_t, base: str, *, # type: ignore[no-any-unimported]
9394
self._default_timeout = timeout
9495
if PATCH_REQUESTS: # pragma: no branch (always enabled in production)
9596
self._session_props["family"] = family
97+
98+
# Ensure that no proxy lookups are done for the UDS pseudo-hostname
99+
#
100+
# I'm well aware of the `.proxies` attribute of the session object: As it turns out,
101+
# setting *that* attribute will *not* bypass system proxy resolution – only the
102+
# per-request keyword-argument can do *that*…!
103+
self._request_proxies = None # type: ty.Optional[ty.Dict[str, str]]
104+
if uds_path:
105+
self._request_proxies = {
106+
"no_proxy": urllib.parse.quote(uds_path, safe=""),
107+
}
96108

97109
def _make_session(self) -> requests.Session: # type: ignore[name-defined]
98110
session = requests.Session() # type: ignore[attr-defined]
@@ -159,6 +171,7 @@ def _request(
159171
headers=headers,
160172
timeout=(timeout if timeout is not None else self._default_timeout),
161173
),
174+
proxies=self._request_proxies,
162175
data=data,
163176
stream=True,
164177
)

test/unit/test_http.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77
TestHttp -- A TCP client for interacting with an IPFS daemon
88
"""
99

10+
import codecs
11+
import errno
1012
import json
13+
import locale
1114
import os
1215
import socket
1316
import tempfile
@@ -35,6 +38,34 @@ def http_server(request):
3538
return server
3639

3740

41+
def make_temp_maxlen_socket_path():
42+
"""Generate a socket filepath of exactly 96 bytes length
43+
44+
When using this as part of the hostname value, the first label will definitely be longer then
45+
the maximum permitted label length of 63 characters (issue found by CI). At the same time
46+
the total binary path length will exceed the portably usable maximum of 96 bytes for the
47+
path length in the `sockaddr_un.sun_path` C socket datastructure (as documented in the
48+
``unix(7)`` Linux man-page).
49+
"""
50+
# The following was inspired by the `tempfile.mktemp` standard library function
51+
temp_dir_bin = tempfile.gettempdir().encode(locale.getpreferredencoding())
52+
with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM, 0) as sock:
53+
for _ in range(tempfile.TMP_MAX):
54+
uds_name_len = (96 - len(temp_dir_bin) - len(os.sep) - len(b".sock"))
55+
uds_name_bin = codecs.encode(os.urandom((uds_name_len + 2) // 2), "hex")[:uds_name_len]
56+
uds_path_bin = os.path.join(temp_dir_bin, uds_name_bin + b".sock")
57+
uds_path_str = uds_path_bin.decode(locale.getpreferredencoding())
58+
if not os.path.exists(uds_path_bin):
59+
try:
60+
sock.bind(uds_path_bin)
61+
except IOError:
62+
continue
63+
else:
64+
return uds_path_str
65+
66+
raise FileExistsError(errno.EEXIST, "No usable temporary filepath found")
67+
68+
3869
@pytest.fixture(scope="module")
3970
def http_server_uds(request):
4071
"""Like :func:`http_server` but will listen on a Unix domain socket instead
@@ -45,7 +76,7 @@ def http_server_uds(request):
4576
if not hasattr(socket, "AF_UNIX"):
4677
pytest.skip("Platform does not support Unix domain sockets")
4778

48-
uds_path = tempfile.mktemp(".sock")
79+
uds_path = make_temp_maxlen_socket_path()
4980
def remove_uds_path():
5081
try:
5182
os.remove(uds_path)
@@ -122,7 +153,6 @@ def test_successful_request_uds(http_client_uds, http_server_uds):
122153

123154
res = http_client_uds.request("/okay")
124155
assert res == b"okay"
125-
print(http_server_uds.requests[0].headers)
126156

127157
def test_generic_failure(http_client, http_server):
128158
"""Tests that a failed http request raises an HTTPError."""

0 commit comments

Comments
 (0)