Repository: aio-libs/aiohttp
Pull request: Fix incorrect rejection of ws:// and wss:// urls
This is because libraries like discord.py needs to use a wss type of url to make a socket to an api service, and it cannot be done outside of aiohttp as it can block the async event loop which can result in undefined behavior.

Test will arrive soon.

<!-- Thank you for your contribution! -->

## What do these changes do?

This fixes the breaking change that breaks the usage of wss style urls inside of api wrappers that requires them in order to start up at all.

## Are there changes in behavior for the user?

The only change is that the wss style urls that previously did not throw in 3.9.5 will now still work in 3.10.0 and 4.0.0 when this gets merged. This is because this change special cases wss links.

## Is it a substantial burden for the maintainers to support this?

I think this is not a substantial burden as this is both a simple change, and prevents breaking applications that expect this entirely while also allowing them to get the latest updates on both features and bug fixes (and possibly also security fixes) vs pinning them forever to 3.9.5 being the last version they can ever use and them eating a security bug that they can never get fixed.

## Related issue number

<!-- Are there any issues opened that will be resolved by merging this change? -->
<!-- Remember to prefix with 'Fixes' if it should close the issue (e.g. 'Fixes #123'). -->
Fixes #8481

## Checklist

Will finish the checklist as I work on this.

- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [ ] Documentation reflects the changes
- [x] If you provide code modification, please add yourself to `CONTRIBUTORS.txt`
  * The format is &lt;Name&gt; &lt;Surname&gt;.
  * Please keep alphabetical order, the file is sorted by names.
- [x] Add a new news fragment into the `CHANGES/` folder
  * name it `<issue_or_pr_num>.<type>.rst` (e.g. `588.bugfix.rst`)
  * if you don't have an issue number, change it to the pull request
    number after creating the PR
    * `.bugfix`: A bug fix for something the maintainers deemed an
      improper undesired behavior that got corrected to match
      pre-agreed expectations.
    * `.feature`: A new behavior, public APIs. That sort of stuff.
    * `.deprecation`: A declaration of future API removals and breaking
      changes in behavior.
    * `.breaking`: When something public is removed in a breaking way.
      Could be deprecated in an earlier release.
    * `.doc`: Notable updates to the documentation structure or build
      process.
    * `.packaging`: Notes for downstreams about unobvious side effects
      and tooling. Changes in the test invocation considerations and
      runtime assumptions.
    * `.contrib`: Stuff that affects the contributor experience. e.g.
      Running tests, building the docs, setting up the development
      environment.
    * `.misc`: Changes that are hard to assign to any of the above
      categories.
  * Make sure to use full sentences with correct case and punctuation,
    for example:
    ```rst
    Fixed issue with non-ascii contents in doctest text files
    -- by :user:`contributor-gh-handle`.
    ```

    Use the past tense or the present tense a non-imperative mood,
    referring to what's changed compared to the last released version
    of this project.


Problem statement:
3.10.0 and 4.0.0 incorrectly reject wss:// urls
### Describe the bug

In discord.py a single wss:// type url is needed to create a websocket using aiohttp to discord's api. This both allows for new and resumed connections.

```
Traceback (most recent call last):
  File "client.py", line 664, in connect
  File "asyncio\tasks.py", line 507, in wait_for
  File "gateway.py", line 374, in from_client
  File "http.py", line 550, in ws_connect
  File "client.py", line 889, in _ws_connect
  File "client.py", line 477, in _request
aiohttp.client_exceptions.NonHttpUrlClientError: wss://gateway.discord.gg/?v=10&encoding=json&compress=zlib-stream

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "__init__.py", line 49, in bot_main
  File "asyncio\runners.py", line 194, in run
  File "asyncio\runners.py", line 118, in run
  File "asyncio\base_events.py", line 721, in run_until_complete
  File "__init__.py", line 38, in run_bot
  File "client.py", line 787, in start
  File "client.py", line 723, in connect
AttributeError: 'NoneType' object has no attribute 'sequence'
Task exception was never retrieved
future: <Task finished name='discord-ext-tasks: VerificationTask.expire_verifications' coro=<Loop._loop() done, defined at __init__.py:202> exception=RuntimeError('Client has not been properly initialised. Please use the login method or asynchronous context manager before calling this method')>
Traceback (most recent call last):
  File "__init__.py", line 204, in _loop
  File "__init__.py", line 188, in _call_loop_function
  File "__init__.py", line 155, in before_task
  File "client.py", line 1149, in wait_until_ready
RuntimeError: Client has not been properly initialised. Please use the login method or asynchronous context manager before calling this method
Unclosed connector
connections: ['[(<aiohttp.client_proto.ResponseHandler object at 0x00000209B9AD7AD0>, 221376.2822786)]']
connector: <aiohttp.connector.TCPConnector object at 0x00000209B9AEABA0>
```

### To Reproduce

1. Install Discord.py & asqlite & audioop-lts
2. Install aiohttp 3.10 (branch) or 4.0.0 (default branch) from source to a manually installed cpython main build (by building installers and running them to install globally)
3. Run discord bot made in discord.py.

### Expected behavior

Everything to work, this should not have broken at all. wss links in discord api is needed through aiohttp because both ``requests`` and any other methods of connecting to discord's api is not going to work inside of a running event loop (it blocks it and is really bad to do that). As such I think it needs expanded to allow wss:// links as a special case.

### Logs/tracebacks

```python-traceback
Traceback (most recent call last):
  File "client.py", line 664, in connect
  File "asyncio\tasks.py", line 507, in wait_for
  File "gateway.py", line 374, in from_client
  File "http.py", line 550, in ws_connect
  File "client.py", line 889, in _ws_connect
  File "client.py", line 477, in _request
aiohttp.client_exceptions.NonHttpUrlClientError: wss://gateway.discord.gg/?v=10&encoding=json&compress=zlib-stream

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "__init__.py", line 49, in bot_main
  File "asyncio\runners.py", line 194, in run
  File "asyncio\runners.py", line 118, in run
  File "asyncio\base_events.py", line 721, in run_until_complete
  File "__init__.py", line 38, in run_bot
  File "client.py", line 787, in start
  File "client.py", line 723, in connect
AttributeError: 'NoneType' object has no attribute 'sequence'
Task exception was never retrieved
future: <Task finished name='discord-ext-tasks: VerificationTask.expire_verifications' coro=<Loop._loop() done, defined at __init__.py:202> exception=RuntimeError('Client has not been properly initialised. Please use the login method or asynchronous context manager before calling this method')>
Traceback (most recent call last):
  File "__init__.py", line 204, in _loop
  File "__init__.py", line 188, in _call_loop_function
  File "__init__.py", line 155, in before_task
  File "client.py", line 1149, in wait_until_ready
RuntimeError: Client has not been properly initialised. Please use the login method or asynchronous context manager before calling this method
Unclosed connector
connections: ['[(<aiohttp.client_proto.ResponseHandler object at 0x00000209B9AD7AD0>, 221376.2822786)]']
connector: <aiohttp.connector.TCPConnector object at 0x00000209B9AEABA0>
```


### Python Version

```console
$ python --version
3.14.0a0
```


### aiohttp Version

```console
$ python -m pip show aiohttp
3.10.0.dev0
```


### multidict Version

```console
$ python -m pip show multidict
6.0.5
```


### yarl Version

```console
$ python -m pip show yarl
1.9.4
```


### OS

Windows

### Related component

Server, Client

### Additional context

I also packed everything into a zip file and embedded python with the ability to load c extensions from zip file.

### Code of Conduct

- [X] I agree to follow the aio-libs Code of Conduct

Hints from issue discussion:
Hi @AraHaan 

Thanks for reporting this issue.

Would you be able to open a PR to an [xfail test](https://docs.pytest.org/en/stable/how-to/skipping.html#xfail-mark-test-functions-as-expected-to-fail) to show this issue?
I already found the fix as well. Changing line 180 in client.py to add ``"wss"`` before the ``""`` will make everything work again.

Will look into making the proper test for it though.
Opened draft pull request with the fix, test will come soon and updated docs and news entry.

Commit message: lint

Patch to review:
diff --git a/CHANGES/8481.bugfix.rst b/CHANGES/8481.bugfix.rst
new file mode 100644
index 00000000000..b185780174e
--- /dev/null
+++ b/CHANGES/8481.bugfix.rst
@@ -0,0 +1,2 @@
+Fixed the incorrect rejection of ``ws://`` and ``wss://`` urls
+-- by :user:` AraHaan`.
diff --git a/aiohttp/client.py b/aiohttp/client.py
index 4c0ad893bbb..25026920206 100644
--- a/aiohttp/client.py
+++ b/aiohttp/client.py
@@ -210,6 +210,8 @@ class ClientTimeout:
 # https://www.rfc-editor.org/rfc/rfc9110#section-9.2.2
 IDEMPOTENT_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "TRACE", "PUT", "DELETE"})
 HTTP_SCHEMA_SET = frozenset({"http", "https", ""})
+WS_SCHEMA_SET = frozenset({"ws", "wss"})
+ALLOWED_PROTOCOL_SCHEMA_SET = HTTP_SCHEMA_SET | WS_SCHEMA_SET
 
 _RetType = TypeVar("_RetType")
 _CharsetResolver = Callable[[ClientResponse, bytes], str]
@@ -452,7 +454,7 @@ async def _request(
         except ValueError as e:
             raise InvalidUrlClientError(str_or_url) from e
 
-        if url.scheme not in HTTP_SCHEMA_SET:
+        if url.scheme not in ALLOWED_PROTOCOL_SCHEMA_SET:
             raise NonHttpUrlClientError(url)
 
         skip_headers = set(self._skip_auto_headers)
diff --git a/tests/conftest.py b/tests/conftest.py
index fb294bd2cad..6310a1dd765 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,10 +1,11 @@
 # type: ignore
 import asyncio
+import base64
 import os
 import socket
 import ssl
 import sys
-from hashlib import md5, sha256
+from hashlib import md5, sha1, sha256
 from pathlib import Path
 from tempfile import TemporaryDirectory
 from typing import Any, List
@@ -13,6 +14,7 @@
 
 import pytest
 
+from aiohttp.http import WS_KEY
 from aiohttp.test_utils import loop_context
 
 try:
@@ -218,3 +220,18 @@ def start_connection():
         spec_set=True,
     ) as start_connection_mock:
         yield start_connection_mock
+
+
+@pytest.fixture
+def key_data():
+    return os.urandom(16)
+
+
+@pytest.fixture
+def key(key_data: Any):
+    return base64.b64encode(key_data)
+
+
+@pytest.fixture
+def ws_key(key: Any):
+    return base64.b64encode(sha1(key + WS_KEY).digest()).decode()
diff --git a/tests/test_client_session.py b/tests/test_client_session.py
index e08e7d10414..82a6198b300 100644
--- a/tests/test_client_session.py
+++ b/tests/test_client_session.py
@@ -467,6 +467,60 @@ async def create_connection(req, traces, timeout):
         c.__del__()
 
 
+@pytest.mark.parametrize("protocol", ["http", "https", "ws", "wss"])
+async def test_ws_connect_allowed_protocols(
+    create_session: Any,
+    create_mocked_conn: Any,
+    protocol: str,
+    ws_key: Any,
+    key_data: Any,
+) -> None:
+    url = URL(f"{protocol}://example.com")
+    req = mock.Mock()
+    req_factory = mock.Mock(return_value=req)
+    resp = mock.Mock()
+    resp.status = 101
+    resp.headers = {
+        hdrs.UPGRADE: "websocket",
+        hdrs.CONNECTION: "upgrade",
+        hdrs.SEC_WEBSOCKET_ACCEPT: ws_key,
+    }
+    resp.url = url
+    resp.cookies = SimpleCookie()
+    resp.start = mock.AsyncMock()
+    req.send = mock.AsyncMock(return_value=resp)
+    session = await create_session(request_class=req_factory)
+
+    connections = []
+    original_connect = session._connector.connect
+
+    async def connect(req, traces, timeout):
+        conn = await original_connect(req, traces, timeout)
+        connections.append(conn)
+        return conn
+
+    async def create_connection(req, traces, timeout):
+        # return self.transport, self.protocol
+        conn = create_mocked_conn()
+        return conn
+
+    session._connector.connect = connect
+    session._connector._create_connection = create_connection
+    session._connector._release = mock.Mock()
+
+    with mock.patch("aiohttp.client.os") as m_os:
+        m_os.urandom.return_value = key_data
+        await session.ws_connect(f"{protocol}://example.com")
+
+    # normally called during garbage collection.  triggers an exception
+    # if the connection wasn't already closed
+    for c in connections:
+        c.close()
+        del c
+
+    await session.close()
+
+
 async def test_cookie_jar_usage(loop: Any, aiohttp_client: Any) -> None:
     req_url = None
 
diff --git a/tests/test_client_ws.py b/tests/test_client_ws.py
index 06cf2a12066..3e469df208f 100644
--- a/tests/test_client_ws.py
+++ b/tests/test_client_ws.py
@@ -15,21 +15,6 @@
 from aiohttp.test_utils import make_mocked_coro
 
 
-@pytest.fixture
-def key_data():
-    return os.urandom(16)
-
-
-@pytest.fixture
-def key(key_data: Any):
-    return base64.b64encode(key_data)
-
-
-@pytest.fixture
-def ws_key(key: Any):
-    return base64.b64encode(hashlib.sha1(key + WS_KEY).digest()).decode()
-
-
 async def test_ws_connect(ws_key: Any, loop: Any, key_data: Any) -> None:
     resp = mock.Mock()
     resp.status = 101


Code review comments:
diff_hunk: @@ -467,6 +467,60 @@ async def create_connection(req, traces, timeout):
         c.__del__()
 
 
+@pytest.mark.parametrize("protocol", ["http", "https", "ws", "wss"])
+async def test_ws_connect_allowed_protocols(
+    create_session: Any,
+    create_mocked_conn: Any,
+    protocol: str,
+    ws_key: Any,
+    key_data: Any,
+) -> None:
+    url = URL(f"{protocol}://example.com")
+    req = mock.Mock()
+    req_factory = mock.Mock(return_value=req)
+    resp = mock.Mock()
+    resp.status = 101
+    resp.headers = {
+        hdrs.UPGRADE: "websocket",
+        hdrs.CONNECTION: "upgrade",
+        hdrs.SEC_WEBSOCKET_ACCEPT: ws_key,
+    }
+    resp.url = url
+    resp.cookies = SimpleCookie()
+    resp.start = mock.AsyncMock()
+    req.send = mock.AsyncMock(return_value=resp)
+    session = await create_session(request_class=req_factory)
+
+    connections = []
+    original_connect = session._connector.connect
+
+    async def connect(req, traces, timeout):
+        conn = await original_connect(req, traces, timeout)
+        connections.append(conn)
+        return conn
+
+    async def create_connection(req, traces, timeout):
+        # return self.transport, self.protocol
+        conn = create_mocked_conn()
+        return conn
+
+    session._connector.connect = connect
+    session._connector._create_connection = create_connection
+    session._connector._release = mock.Mock()
path: tests/test_client_session.py
text: @user1:
This one should be using .patch.object(), right?

@user2:
68ea65fed759e6f8ae69e7146127a30e26e9552f

diff_hunk: @@ -467,6 +467,60 @@ async def create_connection(req, traces, timeout):
         c.__del__()
 
 
+@pytest.mark.parametrize("protocol", ["http", "https", "ws", "wss"])
+async def test_ws_connect_allowed_protocols(
+    create_session: Any,
+    create_mocked_conn: Any,
+    protocol: str,
+    ws_key: Any,
+    key_data: Any,
+) -> None:
+    url = URL(f"{protocol}://example.com")
+    req = mock.Mock()
+    req_factory = mock.Mock(return_value=req)
+    resp = mock.Mock()
+    resp.status = 101
+    resp.headers = {
+        hdrs.UPGRADE: "websocket",
+        hdrs.CONNECTION: "upgrade",
+        hdrs.SEC_WEBSOCKET_ACCEPT: ws_key,
+    }
+    resp.url = url
+    resp.cookies = SimpleCookie()
+    resp.start = mock.AsyncMock()
+    req.send = mock.AsyncMock(return_value=resp)
+    session = await create_session(request_class=req_factory)
+
+    connections = []
+    original_connect = session._connector.connect
+
+    async def connect(req, traces, timeout):
+        conn = await original_connect(req, traces, timeout)
+        connections.append(conn)
+        return conn
+
+    async def create_connection(req, traces, timeout):
+        # return self.transport, self.protocol
+        conn = create_mocked_conn()
+        return conn
path: tests/test_client_session.py
text: @user1:
```suggestion
        return create_mocked_conn()
```

diff_hunk: @@ -467,6 +467,60 @@ async def create_connection(req, traces, timeout):
         c.__del__()
 
 
+@pytest.mark.parametrize("protocol", ["http", "https", "ws", "wss"])
+async def test_ws_connect_allowed_protocols(
+    create_session: Any,
+    create_mocked_conn: Any,
+    protocol: str,
+    ws_key: Any,
+    key_data: Any,
+) -> None:
+    url = URL(f"{protocol}://example.com")
+    req = mock.Mock()
path: tests/test_client_session.py
text: @user1:
For all these things, could we use create_autospec() (ideally with spec_set=True)? Should be able to avoid pretty much all direct Mock/AsyncMock() creations with that.

@user2:
68ea65fed759e6f8ae69e7146127a30e26e9552f

spec_set=True worked for `ClientRequest` but not for `ClientResponse` since it wouldn't accept the cookies when I did that.