Skip to content
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.

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

Fixed TCP clients crash with ssl=True with specific SSL versions + Fixed some tests #381

Merged
merged 1 commit into from
Nov 11, 2024
Merged
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
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
Loading