From ca3570cf3bffda09bebe871d898f3fa7d361db7f Mon Sep 17 00:00:00 2001 From: Karthic Raghupathi Date: Fri, 26 Jun 2026 20:47:32 -0400 Subject: [PATCH 1/6] Handle string data in Originate_Application --- CHANGELOG.md | 1 + pystrix/ami/ami.py | 14 +++++++++++--- pystrix/ami/core.py | 8 +++++--- tests/test_ami_request.py | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c0fd4f..f05e00e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `Manager.send_action` no longer raises a raw `AttributeError` when a concurrent `disconnect()` clears the connection between the liveness check and the send. It now drops the just-registered request and raises `ManagerSocketError` instead (#3). - The FastAGI server no longer prints an unhandled traceback to stderr when a client disconnects during the AGI environment handshake. A caller hanging up, Asterisk aborting the leg, or a bare TCP probe raised `AGISIGPIPEHangup` from the handler and printed a full traceback for a routine event. The handler now ends the request quietly. Errors raised by the script handler itself still propagate (#49). - The FastAGI server sizes its listen backlog without shelling out to `sysctl`. It now requests the maximum backlog and lets the operating system cap it: Unix-like kernels clamp it to the live `somaxconn`, which still tracks a tuned-up limit automatically, and Windows applies the Winsock maximum. This also lifts the platform restriction added in 1.3.0: the server previously raised `NotImplementedError` on any host that was neither Linux nor macOS, and now runs everywhere (#32). +- `Originate_Application` now treats a plain string `data` argument as one application argument instead of splitting it into comma-separated characters (#11). ## [1.3.0] - 2026-06-24 diff --git a/pystrix/ami/ami.py b/pystrix/ami/ami.py index ad59254..5224f3a 100644 --- a/pystrix/ami/ami.py +++ b/pystrix/ami/ami.py @@ -1058,6 +1058,14 @@ def build_request(self, action_id, id_generator, **kwargs): The 'Action' line is always first. """ + + def validate_header(key, value): + if "\r" in key or "\n" in key: + raise ValueError("AMI header keys must not contain CR or LF") + if "\r" in value or "\n" in value: + raise ValueError("AMI header values must not contain CR or LF") + return (key, value) + items = [(KEY_ACTION, self[KEY_ACTION])] for key, value in [ (k, v) for (k, v) in self.items() if k not in (KEY_ACTION, KEY_ACTIONID) @@ -1065,9 +1073,9 @@ def build_request(self, action_id, id_generator, **kwargs): key = str(key) if type(value) in (tuple, list, set, frozenset): for val in value: - items.append((key, str(val))) + items.append(validate_header(key, str(val))) else: - items.append((key, str(value))) + items.append(validate_header(key, str(value))) # Resolve the ActionID using the precedence documented above: an explicit # argument wins, then any value already set on the request, then a generated @@ -1076,7 +1084,7 @@ def build_request(self, action_id, id_generator, **kwargs): if action_id is None: action_id = self[KEY_ACTIONID] if KEY_ACTIONID in self else id_generator() action_id = str(action_id) - items.append((KEY_ACTIONID, action_id)) + items.append(validate_header(KEY_ACTIONID, action_id)) return ( _EOL.join( diff --git a/pystrix/ami/core.py b/pystrix/ami/core.py index 687a307..cb31a61 100644 --- a/pystrix/ami/core.py +++ b/pystrix/ami/core.py @@ -816,9 +816,9 @@ def __init__( `channel` is the destination to be called, expressed as a fully qualified Asterisk channel, like "SIP/test-account@example.org". - `application` is the name of the application to be executed, and `data` is optionally any - parameters to pass to the application, as an ordered sequence (list or tuple) of strings, - escaped as necessary (the ',' character is special). + `application` is the name of the application to be executed, and `data` is optionally a + single string argument or an ordered sequence (list or tuple) of strings to be joined with + commas. The ',' character is special and should be escaped as needed by the caller. `timeout`, if given, is the number of milliseconds to wait before dropping an unanwsered call. If set, the request's timeout value will be set to this number + 2 seconds, removing @@ -842,6 +842,8 @@ def __init__( ) self["Application"] = application if data: + if isinstance(data, str): + data = (data,) self["Data"] = ",".join((str(d) for d in data)) diff --git a/tests/test_ami_request.py b/tests/test_ami_request.py index 316712f..3d9ccbe 100644 --- a/tests/test_ami_request.py +++ b/tests/test_ami_request.py @@ -1,6 +1,9 @@ """Tests for AMI request building (`pystrix.ami.ami._Request`).""" +import pytest + from pystrix.ami.ami import _Request +from pystrix.ami.core import Originate_Application def _build(request, action_id=None, **kwargs): @@ -34,6 +37,39 @@ def test_multi_value_header_expands_to_repeated_lines(): assert "Variable: b=2\r\n" in command +def test_originate_application_treats_string_data_as_one_argument(): + request = Originate_Application("SIP/708", "Playback", "goodbye") + + command, _ = _build(request) + + assert "Data: goodbye\r\n" in command + assert "Data: g,o,o,d,b,y,e\r\n" not in command + + +def test_originate_application_preserves_sequence_data_arguments(): + request = Originate_Application("SIP/708", "Playback", ("goodbye", "noanswer")) + + command, _ = _build(request) + + assert "Data: goodbye,noanswer\r\n" in command + + +def test_rejects_header_value_containing_crlf(): + request = _Request("Originate") + request["Data"] = "goodbye\r\nInjected: x" + + with pytest.raises(ValueError, match="AMI header values must not contain CR or LF"): + _build(request) + + +def test_rejects_header_key_containing_crlf(): + request = _Request("Originate") + request["Data\r\nInjected"] = "goodbye" + + with pytest.raises(ValueError, match="AMI header keys must not contain CR or LF"): + _build(request) + + def test_multi_value_order_preserved_and_blank_line_terminator(): request = _Request("Originate") request["Variable"] = ("a=1", "b=2") From 71feab2ada93c917ddc65b6bea35f370166c143e Mon Sep 17 00:00:00 2001 From: Karthic Raghupathi Date: Sat, 27 Jun 2026 09:47:03 -0400 Subject: [PATCH 2/6] Address PR review findings for Originate data --- CHANGELOG.md | 2 ++ pystrix/ami/ami.py | 11 ++++++++++- pystrix/ami/core.py | 2 ++ tests/test_ami_request.py | 29 +++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f05e00e..fe92a89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The FastAGI server no longer prints an unhandled traceback to stderr when a client disconnects during the AGI environment handshake. A caller hanging up, Asterisk aborting the leg, or a bare TCP probe raised `AGISIGPIPEHangup` from the handler and printed a full traceback for a routine event. The handler now ends the request quietly. Errors raised by the script handler itself still propagate (#49). - The FastAGI server sizes its listen backlog without shelling out to `sysctl`. It now requests the maximum backlog and lets the operating system cap it: Unix-like kernels clamp it to the live `somaxconn`, which still tracks a tuned-up limit automatically, and Windows applies the Winsock maximum. This also lifts the platform restriction added in 1.3.0: the server previously raised `NotImplementedError` on any host that was neither Linux nor macOS, and now runs everywhere (#32). - `Originate_Application` now treats a plain string `data` argument as one application argument instead of splitting it into comma-separated characters (#11). +- `Originate_Application` now rejects `bytes` data with `TypeError` instead of serializing byte values as comma-separated integers (#11). +- AMI request building now rejects header keys and values containing CR or LF, including `Action` and `ActionID`, by raising `ValueError`. ## [1.3.0] - 2026-06-24 diff --git a/pystrix/ami/ami.py b/pystrix/ami/ami.py index 5224f3a..017fcc9 100644 --- a/pystrix/ami/ami.py +++ b/pystrix/ami/ami.py @@ -590,6 +590,8 @@ def send_action(self, request, action_id=None, **kwargs): Raises `ManagerSocketError` if the socket is broken during transmission, including when a concurrent `disconnect()` closes the connection before the request is sent. + Raises `ValueError` if any header key or value contains CR or LF. + This function is thread-safe. """ if not self.is_connected(): @@ -1056,17 +1058,24 @@ def build_request(self, action_id, id_generator, **kwargs): `**kwargs` is a dictionary of additional arguments that may be passed along with the request at submission time. + Raises `ValueError` if any header key or value contains CR or LF. + The 'Action' line is always first. """ def validate_header(key, value): + # AMI v2 uses '\r\n' as the field separator and a bare '\r\n' (blank + # line) as the packet terminator. The protocol defines no escaping + # mechanism, so a literal CR or LF inside a key or value is + # indistinguishable from a field boundary or end-of-packet marker. + # See: https://docs.asterisk.org/Configuration/Interfaces/Asterisk-Manager-Interface-AMI/AMI-v2-Specification/ if "\r" in key or "\n" in key: raise ValueError("AMI header keys must not contain CR or LF") if "\r" in value or "\n" in value: raise ValueError("AMI header values must not contain CR or LF") return (key, value) - items = [(KEY_ACTION, self[KEY_ACTION])] + items = [validate_header(KEY_ACTION, str(self[KEY_ACTION]))] for key, value in [ (k, v) for (k, v) in self.items() if k not in (KEY_ACTION, KEY_ACTIONID) ] + list(kwargs.items()): diff --git a/pystrix/ami/core.py b/pystrix/ami/core.py index cb31a61..ef57550 100644 --- a/pystrix/ami/core.py +++ b/pystrix/ami/core.py @@ -841,6 +841,8 @@ def __init__( self, channel, timeout, callerid, variables, account, async_ ) self["Application"] = application + if isinstance(data, bytes): + raise TypeError("data must be a string or sequence of strings, not bytes") if data: if isinstance(data, str): data = (data,) diff --git a/tests/test_ami_request.py b/tests/test_ami_request.py index 3d9ccbe..2681fac 100644 --- a/tests/test_ami_request.py +++ b/tests/test_ami_request.py @@ -54,6 +54,21 @@ def test_originate_application_preserves_sequence_data_arguments(): assert "Data: goodbye,noanswer\r\n" in command +def test_originate_application_rejects_bytes_data(): + with pytest.raises( + TypeError, match="data must be a string or sequence of strings, not bytes" + ): + Originate_Application("SIP/708", "Playback", b"goodbye") + + +def test_originate_application_omits_empty_string_data(): + request = Originate_Application("SIP/708", "Playback", "") + + command, _ = _build(request) + + assert "Data:" not in command + + def test_rejects_header_value_containing_crlf(): request = _Request("Originate") request["Data"] = "goodbye\r\nInjected: x" @@ -62,6 +77,13 @@ def test_rejects_header_value_containing_crlf(): _build(request) +def test_rejects_action_containing_crlf(): + request = _Request("Ping\r\nInjected: x") + + with pytest.raises(ValueError, match="AMI header values must not contain CR or LF"): + _build(request) + + def test_rejects_header_key_containing_crlf(): request = _Request("Originate") request["Data\r\nInjected"] = "goodbye" @@ -70,6 +92,13 @@ def test_rejects_header_key_containing_crlf(): _build(request) +def test_rejects_action_id_containing_crlf(): + request = _Request("Ping") + + with pytest.raises(ValueError, match="AMI header values must not contain CR or LF"): + _build(request, action_id="safe\r\nInjected: x") + + def test_multi_value_order_preserved_and_blank_line_terminator(): request = _Request("Originate") request["Variable"] = ("a=1", "b=2") From 67ae4f0fe846fc18ce9f4f2b5f206c18096f332b Mon Sep 17 00:00:00 2001 From: Karthic Raghupathi Date: Sat, 27 Jun 2026 10:09:19 -0400 Subject: [PATCH 3/6] Handle bytes-like Originate application data --- CHANGELOG.md | 2 +- pystrix/ami/core.py | 2 +- tests/test_ami_request.py | 25 ++++++++++++++++++++++--- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe92a89..868e776 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The FastAGI server no longer prints an unhandled traceback to stderr when a client disconnects during the AGI environment handshake. A caller hanging up, Asterisk aborting the leg, or a bare TCP probe raised `AGISIGPIPEHangup` from the handler and printed a full traceback for a routine event. The handler now ends the request quietly. Errors raised by the script handler itself still propagate (#49). - The FastAGI server sizes its listen backlog without shelling out to `sysctl`. It now requests the maximum backlog and lets the operating system cap it: Unix-like kernels clamp it to the live `somaxconn`, which still tracks a tuned-up limit automatically, and Windows applies the Winsock maximum. This also lifts the platform restriction added in 1.3.0: the server previously raised `NotImplementedError` on any host that was neither Linux nor macOS, and now runs everywhere (#32). - `Originate_Application` now treats a plain string `data` argument as one application argument instead of splitting it into comma-separated characters (#11). -- `Originate_Application` now rejects `bytes` data with `TypeError` instead of serializing byte values as comma-separated integers (#11). +- `Originate_Application` now rejects bytes-like `data` with `TypeError` instead of serializing byte values as comma-separated integers (#11). - AMI request building now rejects header keys and values containing CR or LF, including `Action` and `ActionID`, by raising `ValueError`. ## [1.3.0] - 2026-06-24 diff --git a/pystrix/ami/core.py b/pystrix/ami/core.py index ef57550..a51ee93 100644 --- a/pystrix/ami/core.py +++ b/pystrix/ami/core.py @@ -841,7 +841,7 @@ def __init__( self, channel, timeout, callerid, variables, account, async_ ) self["Application"] = application - if isinstance(data, bytes): + if isinstance(data, (bytes, bytearray, memoryview)): raise TypeError("data must be a string or sequence of strings, not bytes") if data: if isinstance(data, str): diff --git a/tests/test_ami_request.py b/tests/test_ami_request.py index 2681fac..da4c106 100644 --- a/tests/test_ami_request.py +++ b/tests/test_ami_request.py @@ -54,11 +54,22 @@ def test_originate_application_preserves_sequence_data_arguments(): assert "Data: goodbye,noanswer\r\n" in command -def test_originate_application_rejects_bytes_data(): +@pytest.mark.parametrize( + "data", + [ + b"goodbye", + b"", + bytearray(b"goodbye"), + bytearray(), + memoryview(b"goodbye"), + memoryview(b""), + ], +) +def test_originate_application_rejects_binary_data(data): with pytest.raises( TypeError, match="data must be a string or sequence of strings, not bytes" ): - Originate_Application("SIP/708", "Playback", b"goodbye") + Originate_Application("SIP/708", "Playback", data) def test_originate_application_omits_empty_string_data(): @@ -77,7 +88,7 @@ def test_rejects_header_value_containing_crlf(): _build(request) -def test_rejects_action_containing_crlf(): +def test_rejects_action_value_containing_crlf(): request = _Request("Ping\r\nInjected: x") with pytest.raises(ValueError, match="AMI header values must not contain CR or LF"): @@ -92,6 +103,14 @@ def test_rejects_header_key_containing_crlf(): _build(request) +def test_rejects_multi_value_header_value_containing_crlf(): + request = _Request("Originate") + request["Variable"] = ("a=1\r\nInjected: x", "b=2") + + with pytest.raises(ValueError, match="AMI header values must not contain CR or LF"): + _build(request) + + def test_rejects_action_id_containing_crlf(): request = _Request("Ping") From fb60fcc8df602492aaff353ee5abd8cd3283810a Mon Sep 17 00:00:00 2001 From: Karthic Raghupathi Date: Sat, 27 Jun 2026 10:33:34 -0400 Subject: [PATCH 4/6] Ignore local worktree directories --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 2719fbe..1cae4b5 100644 --- a/.gitignore +++ b/.gitignore @@ -65,6 +65,9 @@ target/ .venv/ venv/ +# Git worktrees +.worktrees/ + # macOS .DS_Store ._* From cf333ad3a362cb798fe041e9737be29e78b14172 Mon Sep 17 00:00:00 2001 From: Karthic Raghupathi Date: Sat, 27 Jun 2026 10:36:33 -0400 Subject: [PATCH 5/6] Reject buffer-protocol Originate data --- pystrix/ami/core.py | 14 ++++++++++++-- tests/test_ami_request.py | 40 +++++++++++++++++++++++++++------------ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/pystrix/ami/core.py b/pystrix/ami/core.py index a51ee93..c80ed61 100644 --- a/pystrix/ami/core.py +++ b/pystrix/ami/core.py @@ -819,6 +819,7 @@ def __init__( `application` is the name of the application to be executed, and `data` is optionally a single string argument or an ordered sequence (list or tuple) of strings to be joined with commas. The ',' character is special and should be escaped as needed by the caller. + Bytes-like objects passed as `data` raise `TypeError`. `timeout`, if given, is the number of milliseconds to wait before dropping an unanwsered call. If set, the request's timeout value will be set to this number + 2 seconds, removing @@ -841,8 +842,17 @@ def __init__( self, channel, timeout, callerid, variables, account, async_ ) self["Application"] = application - if isinstance(data, (bytes, bytearray, memoryview)): - raise TypeError("data must be a string or sequence of strings, not bytes") + if not isinstance(data, str): + try: + data_view = memoryview(data) + except TypeError: + pass + else: + data_view.release() + raise TypeError( + "data must be a string or sequence of strings, " + "not a bytes-like object" + ) if data: if isinstance(data, str): data = (data,) diff --git a/tests/test_ami_request.py b/tests/test_ami_request.py index da4c106..189593c 100644 --- a/tests/test_ami_request.py +++ b/tests/test_ami_request.py @@ -1,5 +1,9 @@ """Tests for AMI request building (`pystrix.ami.ami._Request`).""" +import array +import ctypes +import mmap + import pytest from pystrix.ami.ami import _Request @@ -55,21 +59,33 @@ def test_originate_application_preserves_sequence_data_arguments(): @pytest.mark.parametrize( - "data", + "data_factory", [ - b"goodbye", - b"", - bytearray(b"goodbye"), - bytearray(), - memoryview(b"goodbye"), - memoryview(b""), + pytest.param(lambda: b"goodbye", id="bytes"), + pytest.param(lambda: b"", id="empty-bytes"), + pytest.param(lambda: bytearray(b"goodbye"), id="bytearray"), + pytest.param(lambda: bytearray(), id="empty-bytearray"), + pytest.param(lambda: memoryview(b"goodbye"), id="memoryview"), + pytest.param(lambda: memoryview(b""), id="empty-memoryview"), + pytest.param(lambda: array.array("b", b"hi"), id="array"), + pytest.param(lambda: (ctypes.c_ubyte * 2)(104, 105), id="ctypes-array"), + pytest.param(lambda: mmap.mmap(-1, 2), id="mmap"), ], ) -def test_originate_application_rejects_binary_data(data): - with pytest.raises( - TypeError, match="data must be a string or sequence of strings, not bytes" - ): - Originate_Application("SIP/708", "Playback", data) +def test_originate_application_rejects_binary_data(data_factory): + data = data_factory() + try: + with pytest.raises( + TypeError, + match="data must be a string or sequence of strings, not a bytes-like object", + ): + Originate_Application("SIP/708", "Playback", data) + finally: + if isinstance(data, memoryview): + data.release() + close = getattr(data, "close", None) + if close: + close() def test_originate_application_omits_empty_string_data(): From cd4bfb781f97f95e12487221d209f3e319b3ba4f Mon Sep 17 00:00:00 2001 From: Karthic Raghupathi Date: Sat, 27 Jun 2026 11:16:03 -0400 Subject: [PATCH 6/6] Clarify Originate application data guards --- pystrix/ami/core.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pystrix/ami/core.py b/pystrix/ami/core.py index c80ed61..9224473 100644 --- a/pystrix/ami/core.py +++ b/pystrix/ami/core.py @@ -846,6 +846,7 @@ def __init__( try: data_view = memoryview(data) except TypeError: + # Non-buffer sequences fall through to normal Data joining below. pass else: data_view.release() @@ -854,6 +855,7 @@ def __init__( "not a bytes-like object" ) if data: + # Plain strings skip the buffer probe and need wrapping for join(). if isinstance(data, str): data = (data,) self["Data"] = ",".join((str(d) for d in data))