Skip to content

Add custom ssl_context to aiohttp helper #84573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
starkillerOG wants to merge 4 commits into from
Closed

Add custom ssl_context to aiohttp helper #84573

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a70dc7b
allow to specify custom ssl_context
starkillerOG Dec 26, 2022
50f4f81
fix small errors
starkillerOG Dec 26, 2022
4cebc28
restore verify_ssl argument
starkillerOG Dec 26, 2022
192b6b6
fix error message
starkillerOG Dec 26, 2022
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
2 changes: 1 addition & 1 deletion homeassistant/components/isy994/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ async def async_setup_entry(
https = False
port = host.port or 80
session = aiohttp_client.async_create_clientsession(
hass, verify_ssl=False, cookie_jar=CookieJar(unsafe=True)
hass, ssl_context=False, cookie_jar=CookieJar(unsafe=True)

@bdraco bdraco Dec 26, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The behavior here is a bit unexpected. If ssl_context is false it seems to imply a non secure connection

)
elif host.scheme == "https":
https = True
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/isy994/config_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ async def validate_input(
https = False
port = host.port or HTTP_PORT
session = aiohttp_client.async_create_clientsession(
hass, verify_ssl=False, cookie_jar=CookieJar(unsafe=True)
hass, ssl_context=False, cookie_jar=CookieJar(unsafe=True)
)
elif host.scheme == SCHEME_HTTPS:
https = True
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/unifi/controller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ async def get_unifi_controller(
sslcontext = ssl.create_default_context(cafile=verify_ssl)
else:
session = aiohttp_client.async_create_clientsession(
hass, verify_ssl=verify_ssl, cookie_jar=CookieJar(unsafe=True)
hass, ssl_context=verify_ssl, cookie_jar=CookieJar(unsafe=True)
)

controller = aiounifi.Controller(
Expand Down
25 changes: 13 additions & 12 deletions homeassistant/helpers/aiohttp_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def async_get_clientsession(
@bind_hass
def async_create_clientsession(
hass: HomeAssistant,
verify_ssl: bool = True,
ssl_context: bool | SSLContext = True

@MartinHjelmare MartinHjelmare Dec 26, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a breaking change. Maybe keep the old parameter while adding the new parameter next to the old?

@starkillerOG starkillerOG Dec 26, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is a good idea, schould I log a warning when the old parameter is used that it will be deprecated?

@MartinHjelmare MartinHjelmare Dec 26, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we want to deprecate the old parameter, yes. I'm not sure if we want that. Let's hear what others think first.

@bdraco bdraco Dec 26, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with Martin. We should keep both parameters here. See my comment above as well.

auto_cleanup: bool = True,
**kwargs: Any,
) -> aiohttp.ClientSession:
Expand All @@ -96,7 +96,7 @@ def async_create_clientsession(

clientsession = _async_create_clientsession(
hass,
verify_ssl,
ssl_context,
auto_cleanup_method=auto_cleanup_method,
**kwargs,
)
Expand All @@ -107,14 +107,14 @@ def async_create_clientsession(
@callback
def _async_create_clientsession(
hass: HomeAssistant,
verify_ssl: bool = True,
ssl_context: bool | SSLContext = True
auto_cleanup_method: Callable[[HomeAssistant, aiohttp.ClientSession], None]
| None = None,
**kwargs: Any,
) -> aiohttp.ClientSession:
"""Create a new ClientSession with kwargs, i.e. for cookies."""
clientsession = aiohttp.ClientSession(
connector=_async_get_connector(hass, verify_ssl),
connector=_async_get_connector(hass, ssl_context),
json_serialize=json_dumps,
response_class=HassClientResponse,
**kwargs,
Expand Down Expand Up @@ -239,24 +239,25 @@ def _async_close_websession(event: Event) -> None:

@callback
def _async_get_connector(
hass: HomeAssistant, verify_ssl: bool = True
hass: HomeAssistant, ssl_context: bool | SSLContext = True
) -> aiohttp.BaseConnector:
"""Return the connector pool for aiohttp.

This method must be run in the event loop.
"""
key = DATA_CONNECTOR if verify_ssl else DATA_CONNECTOR_NOTVERIFY
key = None
if isinstance(ssl_context, bool):
key = DATA_CONNECTOR if verify_ssl else DATA_CONNECTOR_NOTVERIFY

if key in hass.data:
return cast(aiohttp.BaseConnector, hass.data[key])
if key in hass.data:
return cast(aiohttp.BaseConnector, hass.data[key])

if verify_ssl:
if ssl_context is True:
ssl_context: bool | SSLContext = ssl_util.client_context()
else:
ssl_context = False

connector = aiohttp.TCPConnector(enable_cleanup_closed=True, ssl=ssl_context)
hass.data[key] = connector
if key is not None:
hass.data[key] = connector

async def _async_close_connector(event: Event) -> None:
"""Close connector pool."""
Expand Down