Skip to content

Commit

Permalink
Fixed TCP clients crash with ssl=True with specific SSL versions + Fi…
Browse files Browse the repository at this point in the history
…xed some tests
  • Loading branch information
francis-clairicia committed Nov 11, 2024
1 parent 4e3fb8e commit a521524
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 54 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,15 @@ jobs:
&& (github.event_name != 'pull_request' || github.event.pull_request.draft != true)
runs-on: ubuntu-24.04

name: type-hinting (freebsd-14)
name: type-hinting (freebsd-14.1)
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Launch checks
uses: ./.github/actions/freebsd-vm
with:
timeout-minutes: 10
release: '14.1'
python-version: '3.11'
run: |
Expand Down
9 changes: 2 additions & 7 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,6 @@ jobs:
if: hashFiles('.coverage.*') != '' # Rudimentary `file.exists()`
continue-on-error: true
run: tox run -f coverage
# Currently, it is not possible to send several files with per-file tags.
# This is why the step is copy-paste twice.
# Issue: https://github.com/codecov/codecov-action/issues/1522
- name: Upload (unit tests) coverage to codecov
if: hashFiles('coverage.unit.xml') != '' # Rudimentary `file.exists()`
uses: codecov/codecov-action@v4
Expand Down Expand Up @@ -122,8 +119,9 @@ jobs:
strategy:
fail-fast: false
matrix:
# TODO: Add test with other python version
# TODO: Add test with other python versions
# c.f. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271673
python_version: ['3.11']
include:
- python_version: '3.11'
tox_py: py311
Expand All @@ -146,9 +144,6 @@ jobs:
- name: Check files in workspace
if: always()
run: ls -lA
# Currently, it is not possible to send several files with per-file tags.
# This is why the step is copy-paste twice.
# Issue: https://github.com/codecov/codecov-action/issues/1522
- name: Upload (unit tests) coverage to codecov
if: hashFiles('coverage.unit.xml') != '' # Rudimentary `file.exists()`
uses: codecov/codecov-action@v4
Expand Down
3 changes: 2 additions & 1 deletion src/easynetwork/clients/async_tcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ def __init__(
assert isinstance(ssl, _ssl_module.SSLContext) # nosec assert_used
if server_hostname is not None and not server_hostname:
ssl.check_hostname = False
ssl.options &= ~_ssl_module.OP_IGNORE_UNEXPECTED_EOF
with contextlib.suppress(AttributeError):
ssl.options &= ~_ssl_module.OP_IGNORE_UNEXPECTED_EOF
else:
if server_hostname is not None:
raise ValueError("server_hostname is only meaningful with ssl")
Expand Down
3 changes: 2 additions & 1 deletion src/easynetwork/clients/tcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ def __init__(
assert isinstance(ssl, _ssl_module.SSLContext) # nosec assert_used
if not server_hostname:
ssl.check_hostname = False
ssl.options &= ~_ssl_module.OP_IGNORE_UNEXPECTED_EOF
with contextlib.suppress(AttributeError):
ssl.options &= ~_ssl_module.OP_IGNORE_UNEXPECTED_EOF
if not server_hostname:
server_hostname = None

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ async def main() -> str | None:


@pytest.mark.asyncio
@pytest.mark.flaky(retries=3, delay=0)
@pytest.mark.flaky(retries=3, delay=0.1)
class TestAsyncioBackend:
@pytest.fixture
@staticmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ async def main() -> str | None:


@pytest.mark.feature_trio(async_test_auto_mark=True)
@pytest.mark.flaky(retries=3, delay=0)
@pytest.mark.flaky(retries=3, delay=0.1)
class TestTrioBackend:

@pytest.fixture(scope="class")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,30 +237,16 @@ async def test____iter_received_packets____yields_available_packets_until_eof(
event_loop.call_soon(server.close)
assert [p async for p in client.iter_received_packets(timeout=None)] == ["A", "B", "C", "D", "E"]

async def test____iter_received_packets____yields_available_packets_within_timeout(
async def test____iter_received_packets____yields_available_packets_until_timeout(
self,
client: AsyncTCPNetworkClient[str, str],
server: Socket,
) -> None:
event_loop = asyncio.get_running_loop()

async def send_coro() -> None:
await event_loop.sock_sendall(server, b"A\n")
await asyncio.sleep(0.1)
await event_loop.sock_sendall(server, b"B\n")
await asyncio.sleep(0.4)
await event_loop.sock_sendall(server, b"C\n")
await asyncio.sleep(0.2)
await event_loop.sock_sendall(server, b"D\n")
await asyncio.sleep(0.5)
await event_loop.sock_sendall(server, b"E\n")

send_task = asyncio.create_task(send_coro())
try:
assert [p async for p in client.iter_received_packets(timeout=1)] == ["A", "B", "C", "D"]
finally:
send_task.cancel()
await asyncio.wait({send_task})
await event_loop.sock_sendall(server, b"A\nB\nC\nD\nE\n")
await event_loop.sock_sendall(server, b"F\n")
assert [p async for p in client.iter_received_packets(timeout=1)] == ["A", "B", "C", "D", "E", "F"]

async def test____get_local_address____consistency(self, socket_family: int, client: AsyncTCPNetworkClient[str, str]) -> None:
address = client.get_local_address()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,28 +183,16 @@ async def test____iter_received_packets____yields_available_packets_until_close(
close_task.cancel()
await asyncio.wait({close_task})

async def test____iter_received_packets____yields_available_packets_within_given_timeout(
async def test____iter_received_packets____yields_available_packets_until_timeout(
self,
client: AsyncUDPNetworkClient[str, str],
server: DatagramEndpoint,
) -> None:
async def send_coro() -> None:
await server.sendto(b"A", client.get_local_address())
await asyncio.sleep(0.1)
await server.sendto(b"B", client.get_local_address())
await asyncio.sleep(0.4)
await server.sendto(b"C", client.get_local_address())
await asyncio.sleep(0.2)
await server.sendto(b"D", client.get_local_address())
await asyncio.sleep(0.5)
await server.sendto(b"E", client.get_local_address())

send_task = asyncio.create_task(send_coro())
try:
assert [p async for p in client.iter_received_packets(timeout=1)] == ["A", "B", "C", "D"]
finally:
send_task.cancel()
await asyncio.wait({send_task})
for p in [b"A", b"B", b"C", b"D", b"E", b"F"]:
await server.sendto(p, client.get_local_address())

# NOTE: Comparison using set because equality check does not verify order
assert {p async for p in client.iter_received_packets(timeout=1)} == {"A", "B", "C", "D", "E", "F"}

async def test____get_local_address____consistency(self, socket_family: int, client: AsyncUDPNetworkClient[str, str]) -> None:
address = client.get_local_address()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ async def server(
if server_ssl_context is None:
if use_ssl:
pytest.skip("trustme is not installed")
else:
elif hasattr(ssl, "OP_IGNORE_UNEXPECTED_EOF"):
# Remove this option for non-regression
server_ssl_context.options &= ~ssl.OP_IGNORE_UNEXPECTED_EOF

Expand Down Expand Up @@ -1214,8 +1214,9 @@ async def test____serve_forever____suppress_ssl_ragged_eof_errors(
) -> None:
caplog.set_level(logging.WARNING, LOGGER.name)

# This test must fail if this option was not unset when creating the server
assert (server_ssl_context.options & ssl.OP_IGNORE_UNEXPECTED_EOF) == 0
if hasattr(ssl, "OP_IGNORE_UNEXPECTED_EOF"):
# This test must fail if this option was not unset when creating the server
assert (server_ssl_context.options & ssl.OP_IGNORE_UNEXPECTED_EOF) == 0

from easynetwork.lowlevel.api_async.transports.tls import AsyncTLSStreamTransport

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@

import pytest

from ....tools import TimeTest
from ....tools import PlatformMarkers, TimeTest

ClientType: TypeAlias = AbstractNetworkClient[str, str]


pytestmark = [
pytest.mark.flaky(retries=3, delay=1),
PlatformMarkers.skipif_platform_bsd_because("test failures are all too frequent on CI", skip_only_on_ci=True),
]


Expand Down
4 changes: 2 additions & 2 deletions tests/functional_test/test_serializers/test_msgpack.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ def complete_data_for_incremental_deserialize(complete_data: bytes) -> bytes:
@pytest.fixture(scope="class")
@staticmethod
def invalid_complete_data(complete_data: bytes) -> bytes:
return complete_data[:-1] # Extra data error
return complete_data[:-1] # Missing data error

@pytest.fixture(scope="class")
@staticmethod
def invalid_partial_data_extra_data() -> tuple[bytes, bytes]:
return (b"remaining_data", b"")
pytest.skip("Cannot be tested")
7 changes: 7 additions & 0 deletions tests/unit_test/test_async/test_api/test_client/test_tcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -637,10 +637,12 @@ async def test____dunder_init____ssl____server_hostname____required_if_socket_is
)

@pytest.mark.parametrize("use_socket", [False, True], ids=lambda p: f"use_socket=={p}")
@pytest.mark.parametrize("OP_IGNORE_UNEXPECTED_EOF", [False, True], ids=lambda p: f"OP_IGNORE_UNEXPECTED_EOF=={p}")
async def test____dunder_init____ssl____create_default_context(
self,
async_finalizer: AsyncFinalizer,
use_socket: bool,
OP_IGNORE_UNEXPECTED_EOF: bool,
remote_address: tuple[str, int],
mock_backend: MagicMock,
mock_tcp_socket: MagicMock,
Expand All @@ -649,9 +651,14 @@ async def test____dunder_init____ssl____create_default_context(
mock_ssl_create_default_context: MagicMock,
mock_tls_wrap_transport: AsyncMock,
mock_stream_socket_adapter: MagicMock,
monkeypatch: pytest.MonkeyPatch,
mocker: MockerFixture,
) -> None:
# Arrange
if not OP_IGNORE_UNEXPECTED_EOF:
monkeypatch.delattr("ssl.OP_IGNORE_UNEXPECTED_EOF", raising=False)
elif not hasattr(ssl, "OP_IGNORE_UNEXPECTED_EOF"):
pytest.skip("ssl.OP_IGNORE_UNEXPECTED_EOF not defined")

# Act
client: AsyncTCPNetworkClient[Any, Any]
Expand Down
8 changes: 8 additions & 0 deletions tests/unit_test/test_sync/test_client/test_tcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import contextlib
import errno
import os
import ssl
from collections.abc import Iterator
from selectors import EVENT_READ, EVENT_WRITE
from socket import AF_INET6, IPPROTO_TCP, SHUT_RDWR, SHUT_WR, SO_KEEPALIVE, SOL_SOCKET, TCP_NODELAY
Expand Down Expand Up @@ -760,18 +761,25 @@ def test____dunder_init____ssl____server_hostname____no_host_to_use(

@pytest.mark.parametrize("use_ssl", ["USE_SSL"], indirect=True)
@pytest.mark.parametrize("use_socket", [False, True], ids=lambda p: f"use_socket=={p}")
@pytest.mark.parametrize("OP_IGNORE_UNEXPECTED_EOF", [False, True], ids=lambda p: f"OP_IGNORE_UNEXPECTED_EOF=={p}")
def test____dunder_init____ssl____create_default_context(
self,
request: pytest.FixtureRequest,
use_socket: bool,
OP_IGNORE_UNEXPECTED_EOF: bool,
remote_address: tuple[str, int],
mock_ssl_context: MagicMock,
mock_tcp_socket: MagicMock,
mock_ssl_create_default_context: MagicMock,
mock_stream_protocol: MagicMock,
server_hostname: Any,
monkeypatch: pytest.MonkeyPatch,
) -> None:
# Arrange
if not OP_IGNORE_UNEXPECTED_EOF:
monkeypatch.delattr("ssl.OP_IGNORE_UNEXPECTED_EOF", raising=False)
elif not hasattr(ssl, "OP_IGNORE_UNEXPECTED_EOF"):
pytest.skip("ssl.OP_IGNORE_UNEXPECTED_EOF not defined")

# Act
client: TCPNetworkClient[Any, Any]
Expand Down

0 comments on commit a521524

Please sign in to comment.