Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion cloudinit/url_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import os
import threading
import time
import socket
from concurrent.futures import ThreadPoolExecutor, TimeoutError, as_completed
from email.utils import parsedate
from functools import partial
Expand Down Expand Up @@ -45,6 +46,15 @@
REDACTED = "REDACTED"
ExceptionCallback = Optional[Callable[["UrlError"], bool]]

class HTTPAdapterWithSocketOptions(requests.adapters.HTTPAdapter):
def __init__(self, *args, **kwargs):
self.socket_options = kwargs.pop("socket_options", None)
super(HTTPAdapterWithSocketOptions, self).__init__(*args, **kwargs)

def init_poolmanager(self, *args, **kwargs):
if self.socket_options is not None:
kwargs["socket_options"] = self.socket_options
super(HTTPAdapterWithSocketOptions, self).init_poolmanager(*args, **kwargs)

def _cleanurl(url):
parsed_url = list(urlparse(url, scheme="http"))
Expand Down Expand Up @@ -499,8 +509,10 @@ def readurl(
downloaded.
"""
url = _cleanurl(url)
parsed_url = util.url_parser_plus(url)
iface = parsed_url.iface
req_args = {
"url": url,
"url": parsed_url.request_url,
"stream": stream,
}
req_args.update(_get_ssl_args(url, ssl_details))
Expand Down Expand Up @@ -533,12 +545,19 @@ def readurl(
if session is None:
session = requests.Session()

if iface is not None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove all of the changes to the top of this function and add something like this above the if condition here:

    parsed_url = urlparse(url)
    iface = parsed_url.hostname.split("%")[1] if parsed_url.hostname else None

adapter = HTTPAdapterWithSocketOptions(socket_options=[(socket.SOL_SOCKET, socket.SO_BINDTODEVICE, iface.encode('utf-8'))])
for scheme in ('http://', 'https://'):
session.mount(scheme, adapter)

# Handle retrying ourselves since the built-in support
# doesn't handle sleeping between tries...
for i in count():
if headers_cb:
headers = headers_cb(url)

headers["Host"] = parsed_url.hostname

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this line needed?


if "User-Agent" not in headers:
headers["User-Agent"] = user_agent

Expand Down
58 changes: 53 additions & 5 deletions cloudinit/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
Union,
cast,
)
from urllib import parse
from urllib.parse import urlparse, unquote

import yaml

Expand Down Expand Up @@ -92,6 +92,54 @@
TRUE_STRINGS = ("true", "1", "on", "yes")
FALSE_STRINGS = ("off", "0", "no", "false")

class URLParserPlus:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than a whole bucket of new parsing, I'd rather we just extract the iface from already parsed host. See my comment in url_helper.py. I don't think we need this new class.

def __init__(self, url):
parsed = urlparse(url)
self.original_url = url
self.scheme = parsed.scheme.lower()
self.default_ports = {
'http': 80,
'https': 443,
'ftp': 21,
'sftp': 22
}
self.port = parsed.port or self.default_ports.get(self.scheme)
self.iface = None
self.hostname = parsed.hostname
self.path = parsed.path

if self.scheme == 'file':
self.host = None
self.port = None
self.request_url = self.original_url
return

raw_netloc = parsed.netloc
match = re.match(r'\[(.*?)\](?::(\d+))?', raw_netloc)
if match:
full_host = unquote(match.group(1))
if '%' in full_host:
self.hostname, self.iface = full_host.split('%', 1)
else:
self.hostname = full_host
elif self.hostname and '%' in self.hostname:
self.hostname, self.iface = self.hostname.split('%', 1)

host = self.hostname
if ':' in host: # IPv6 literal
host = f'[{host}]'
netloc = f"{host}:{self.port}" if self.port else host
self.request_url = f"{self.scheme}://{netloc}{self.path}"

def __repr__(self):
return (
f"URLParserPlus(original_url={self.original_url}, scheme={self.scheme}, "
f"hostname={self.hostname}, port={self.port}, iface={self.iface}, "
f"path={self.path}, request_url={self.request_url})"
)

def url_parser_plus(url):
return URLParserPlus(url)

def kernel_version():
return tuple(map(int, os.uname().release.split(".")[:2]))
Expand Down Expand Up @@ -1292,8 +1340,8 @@ def is_resolvable(url) -> bool:
be resolved inside the search list.
"""
global _DNS_REDIRECT_IP
parsed_url = parse.urlparse(url)
name = parsed_url.hostname
parsed_url = url_parser_plus(url)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why any of the changes to this function are needed. The call to net.is_ip_address should return true for any ipv6 address, including those that are interface scoped. Since we strip the [] before passing it in, there shouldn't be a problem here.

If that fails, then we should have an actual non-IP hostname, so the call to socket.getaddrinfo also shouldn't need any changes.

Am I missing something?

host = parsed_url.hostname
if _DNS_REDIRECT_IP is None:
badips = set()
badnames = (
Expand All @@ -1320,9 +1368,9 @@ def is_resolvable(url) -> bool:
try:
# ip addresses need no resolution
with suppress(ValueError):
if net.is_ip_address(parsed_url.netloc.strip("[]")):
if net.is_ip_address(host):
return True
result = socket.getaddrinfo(name, None)
result = socket.getaddrinfo(host, None)
# check first result's sockaddr field
addr = result[0][4][0]
return addr not in _DNS_REDIRECT_IP
Expand Down
Loading