diff --git a/docs/history.rst b/docs/history.rst index 04dce5c..f02c79a 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -1,9 +1,18 @@ History ======= +3.0.2 + * bugfix: :meth:`~telnetlib3.stream_writer.TelnetWriter.request_charset` raised :exc:`TypeError`, + :ghissue:`128`. Offer callbacks (no-arg, returning a list of items to propose) are now + separated from send callbacks (which respond to received requests) via new + :meth:`~telnetlib3.stream_writer.TelnetWriter.set_ext_offer_callback` method. + 3.0.1 * change: Unused client argument ``gmcp_log`` removed. * new: MCCP2 and MCCP3. Both client and server ends passively support if requested, and request support by --compression or deny support by --no-compression. + * new: :meth:`~telnetlib3.client.TelnetClient.on_request_charset` and + :meth:`~telnetlib3.client.TelnetClient.on_request_environ` offer callbacks + on the client, symmetric with the existing server-side callbacks. 3.0.0 * change: :attr:`~telnetlib3.client_base.BaseClient.connect_minwait` default diff --git a/pyproject.toml b/pyproject.toml index 9b0786b..89c7aab 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "telnetlib3" -version = "3.0.1" ## Keep in sync with telnetlib3.accessories.get_version() ! +version = "3.0.2" # Keep in sync with telnetlib3/accessories.py::get_version ! description = " Python Telnet server and client CLI and Protocol library" readme = "README.rst" license = "ISC" diff --git a/telnetlib3/accessories.py b/telnetlib3/accessories.py index c414e20..71524ef 100644 --- a/telnetlib3/accessories.py +++ b/telnetlib3/accessories.py @@ -42,7 +42,7 @@ def get_version() -> str: """Return the current version of telnetlib3.""" - return "3.0.1" # keep in sync with pyproject.toml and docs/conf.py !! + return "3.0.2" # keep in sync with pyproject.toml ! def encoding_from_lang(lang: str) -> Optional[str]: diff --git a/telnetlib3/client.py b/telnetlib3/client.py index 0b59f9b..28e8166 100755 --- a/telnetlib3/client.py +++ b/telnetlib3/client.py @@ -136,6 +136,14 @@ def connection_made(self, transport: asyncio.BaseTransport) -> None: ): self.writer.set_ext_send_callback(opt, func) + # Offer callbacks define what to include in outgoing requests + # (e.g. what charsets to offer in SB CHARSET REQUEST). + for opt, offer_func in ( + (CHARSET, self.on_request_charset), + (NEW_ENVIRON, self.on_request_environ), + ): + self.writer.set_ext_offer_callback(opt, offer_func) + # Override the default handle_will method to detect when both sides support CHARSET # Store the original only on first connection to prevent chain growth on reconnect. if not hasattr(self.writer, "_original_handle_will"): @@ -372,6 +380,28 @@ def send_charset(self, offered: List[str]) -> str: self.log.warning("No suitable encoding offered by server: %s", offered) return "" + def on_request_charset(self) -> List[str]: + """ + Offer callback for client-initiated CHARSET REQUEST, :rfc:`2066`. + + Called by :meth:`~.TelnetWriter.request_charset` to determine which + character sets the client offers to the server. + + :returns: List of charset name strings to offer. + """ + return ["UTF-8", "LATIN1", "US-ASCII"] + + def on_request_environ(self) -> List[str]: + """ + Offer callback for client-initiated NEW_ENVIRON SEND, :rfc:`1572`. + + Called by :meth:`~.TelnetWriter.request_environ` to determine which + environment variable names the client requests from the server. + + :returns: List of environment variable names to request. + """ + return [] + def send_naws(self) -> Tuple[int, int]: """ Callback for responding to NAWS requests. diff --git a/telnetlib3/client_base.py b/telnetlib3/client_base.py index 940b2d6..46134b2 100644 --- a/telnetlib3/client_base.py +++ b/telnetlib3/client_base.py @@ -68,9 +68,9 @@ def __init__( self._limit = limit # MCCP2: server→client decompression - self._mccp2_decompressor: Optional[zlib.Decompress] = None + self._mccp2_decompressor: Optional[zlib._Decompress] = None # MCCP3: client→server compression - self._mccp3_compressor: Optional[zlib.Compress] = None + self._mccp3_compressor: Optional[zlib._Compress] = None self._mccp3_orig_write: Any = None # High-throughput receive pipeline @@ -470,7 +470,7 @@ def compressed_write(data: bytes) -> None: else: orig_write(data) - transport.write = compressed_write # type: ignore[assignment] + transport.write = compressed_write # type: ignore[method-assign] self._mccp3_orig_write = orig_write self.log.debug("MCCP3 compression started (client→server)") diff --git a/telnetlib3/server.py b/telnetlib3/server.py index ca88b8a..123f4e5 100755 --- a/telnetlib3/server.py +++ b/telnetlib3/server.py @@ -167,12 +167,12 @@ def connection_made(self, transport: asyncio.BaseTransport) -> None: for tel_opt, callback_fn in _ext_callbacks: self.writer.set_ext_callback(tel_opt, callback_fn) - # Wire up a callbacks that return definitions for requests. + # Wire up offer callbacks that return definitions for outgoing requests. for tel_opt, callback_fn in [ (NEW_ENVIRON, self.on_request_environ), (CHARSET, self.on_request_charset), ]: - self.writer.set_ext_send_callback(tel_opt, callback_fn) + self.writer.set_ext_offer_callback(tel_opt, callback_fn) def data_received(self, data: bytes) -> None: """Process received data and reset timeout timer.""" @@ -208,7 +208,7 @@ def compressed_write(data: bytes) -> None: else: orig_write(data) - transport.write = compressed_write # type: ignore[assignment] + transport.write = compressed_write # type: ignore[method-assign] self._mccp2_orig_write = orig_write self.writer.mccp2_active = True logger.debug("MCCP2 compression started (server→client)") @@ -217,6 +217,7 @@ def _mccp2_end(self) -> None: """Stop MCCP2 compression, flush Z_FINISH.""" if self._mccp2_compressor is not None: try: + assert self._mccp2_orig_write is not None self._mccp2_orig_write(self._mccp2_compressor.flush(zlib.Z_FINISH)) except zlib.error as exc: logger.debug("MCCP2 Z_FINISH flush error: %s", exc) diff --git a/telnetlib3/server_base.py b/telnetlib3/server_base.py index d12acbe..266bf07 100644 --- a/telnetlib3/server_base.py +++ b/telnetlib3/server_base.py @@ -30,7 +30,7 @@ class BaseServer(TelnetProtocolBase, asyncio.streams.FlowControlMixin, asyncio.P _check_later = None _rx_bytes = 0 _tx_bytes = 0 - _mccp3_decompressor: Optional[zlib.Decompress] = None + _mccp3_decompressor: Optional[zlib._Decompress] = None def __init__( self, diff --git a/telnetlib3/stream_writer.py b/telnetlib3/stream_writer.py index 44d3489..fd107d0 100644 --- a/telnetlib3/stream_writer.py +++ b/telnetlib3/stream_writer.py @@ -395,9 +395,19 @@ def __init__( ): self.set_ext_send_callback(cmd=ext_cmd, func=getattr(self, f"handle_send_{key}")) + # Offer callbacks: used by request_charset() and request_environ() + # to get the list of items to offer/request. Separate from + # _ext_send_callback which is used to *respond* to received requests. + self._ext_offer_callback: dict[bytes, Callable[..., Any]] = {} + for ext_cmd, key in ((CHARSET, "charset"), (NEW_ENVIRON, "environ")): - _cbname = "handle_send_server_" if self.server else "handle_send_client_" - self.set_ext_send_callback(cmd=ext_cmd, func=getattr(self, _cbname + key)) + # The "server" default handlers take no args and return lists + # (what to offer/request). The "client" handlers take args + # (respond to received offers). + self.set_ext_offer_callback( + cmd=ext_cmd, func=getattr(self, f"handle_send_server_{key}") + ) + self.set_ext_send_callback(cmd=ext_cmd, func=getattr(self, f"handle_send_client_{key}")) @property def connection_closed(self) -> bool: @@ -430,6 +440,7 @@ def close(self) -> None: # break circular refs self._ext_callback.clear() self._ext_send_callback.clear() + self._ext_offer_callback.clear() self._slc_callback.clear() self._iac_callback.clear() self._protocol = None @@ -1148,7 +1159,7 @@ def request_charset(self) -> bool: self.log.debug("cannot send SB CHARSET REQUEST, request pending.") return False - codepages = self._ext_send_callback[CHARSET]() + codepages = self._ext_offer_callback[CHARSET]() sep = " " response: collections.deque[bytes] = collections.deque() @@ -1171,7 +1182,7 @@ def request_environ(self) -> bool: self.log.debug("cannot send SB NEW_ENVIRON SEND IS without receipt of WILL NEW_ENVIRON") return False - request_list = self._ext_send_callback[NEW_ENVIRON]() + request_list = self._ext_offer_callback[NEW_ENVIRON]() if not request_list: self.log.debug( @@ -1512,6 +1523,26 @@ def set_ext_send_callback(self, cmd: bytes, func: Callable[..., Any]) -> None: """ self._ext_send_callback[cmd] = func + def set_ext_offer_callback(self, cmd: bytes, func: Callable[..., Any]) -> None: + """ + Register callback for building outgoing sub-negotiation requests. + + Unlike :meth:`set_ext_send_callback` (which responds to *received* + requests), this callback is invoked with **no arguments** and must + return a list describing what to offer or request. + + :param cmd: Telnet option byte. + :param func: Callable returning a list: + + * ``CHARSET``: return a list of charset name strings to offer + in an outgoing ``SB CHARSET REQUEST``, :rfc:`2066`. + + * ``NEW_ENVIRON``: return a list of environment variable name + strings (or the special ``VAR``/``USERVAR`` bytes) to request + in an outgoing ``SB NEW_ENVIRON SEND``, :rfc:`1572`. + """ + self._ext_offer_callback[cmd] = func + def set_ext_callback(self, cmd: bytes, func: Callable[..., Any]) -> None: """ Register ``func`` as callback for receipt of ``cmd`` negotiation. diff --git a/telnetlib3/tests/test_charset.py b/telnetlib3/tests/test_charset.py index f6e26ba..27bb5e0 100644 --- a/telnetlib3/tests/test_charset.py +++ b/telnetlib3/tests/test_charset.py @@ -182,7 +182,7 @@ def test_server_sends_do_and_will_charset(): def test_client_do_will_then_server_will_allows_client_request(): """Test scenario from logfile: DO->WILL then server WILL allows client to send SB REQUEST.""" wc, tc, _ = new_writer(server=False, client=True) - wc.set_ext_send_callback(CHARSET, lambda: ["UTF-8"]) + wc.set_ext_offer_callback(CHARSET, lambda: ["UTF-8"]) # Simulate server DO CHARSET # Note: handle_do() returns True but local_option[...] is set by the caller @@ -208,11 +208,11 @@ def test_bidirectional_charset_both_sides_can_request(): """Test that both server and client can initiate CHARSET REQUEST when both have WILL/DO.""" # Server side ws, ts, _ = new_writer(server=True) - ws.set_ext_send_callback(CHARSET, lambda: ["UTF-8", "ASCII"]) + ws.set_ext_offer_callback(CHARSET, lambda: ["UTF-8", "ASCII"]) # Client side wc, tc, _ = new_writer(server=False, client=True) - wc.set_ext_send_callback(CHARSET, lambda: ["UTF-8"]) + wc.set_ext_offer_callback(CHARSET, lambda: ["UTF-8"]) # Simulate full negotiation: server DO, client WILL, server WILL, client DO ws.remote_option[CHARSET] = True # client sent WILL @@ -234,7 +234,7 @@ def test_charset_request_response_cycle(): # Server initiates REQUEST ws, ts, _ = new_writer(server=True) ws.remote_option[CHARSET] = True - ws.set_ext_send_callback(CHARSET, lambda: ["UTF-8", "ASCII"]) + ws.set_ext_offer_callback(CHARSET, lambda: ["UTF-8", "ASCII"]) assert ws.request_charset() is True request_frame = ts.writes[-1] @@ -265,7 +265,7 @@ def test_server_sends_will_charset_after_client_will(): # Verify server also called request_charset as usual # (this is tested by checking if it would send a request, # but we need to set up the callback first) - ws.set_ext_send_callback(CHARSET, lambda: ["UTF-8"]) + ws.set_ext_offer_callback(CHARSET, lambda: ["UTF-8"]) # Clear previous writes to test just the request ts.writes.clear() @@ -401,3 +401,67 @@ def test_charset_accepted_sets_force_binary_on_accepting_side(): assert w.environ_encoding == "UTF-8" assert p.force_binary is True + + +def test_client_request_charset_uses_offer_callback(): + """Client request_charset() must use offer callback, not send callback.""" + wc, tc, _ = new_writer(server=False, client=True) + wc.local_option[CHARSET] = True + wc.remote_option[CHARSET] = True + + wc.set_ext_offer_callback(CHARSET, lambda: ["UTF-8", "CP437"]) + wc.set_ext_send_callback(CHARSET, lambda offered: offered[0]) + + assert wc.request_charset() is True + frame = tc.writes[-1] + assert frame.startswith(IAC + SB + CHARSET + REQUEST) + assert b"UTF-8" in frame + assert b"CP437" in frame + + +def test_server_request_charset_uses_offer_callback(): + """Server request_charset() must use offer callback, not send callback.""" + ws, ts, _ = new_writer(server=True) + ws.remote_option[CHARSET] = True + + ws.set_ext_offer_callback(CHARSET, lambda: ["UTF-8", "ASCII"]) + ws.set_ext_send_callback(CHARSET, lambda offered: offered[0]) + + assert ws.request_charset() is True + frame = ts.writes[-1] + assert frame.startswith(IAC + SB + CHARSET + REQUEST) + assert b"UTF-8" in frame + + +def test_handle_sb_charset_request_uses_send_callback(): + """Receiving SB CHARSET REQUEST must use send callback (not offer).""" + wc, tc, _ = new_writer(server=False, client=True) + wc.local_option[CHARSET] = True + wc.remote_option[CHARSET] = True + + offer_called = [] + wc.set_ext_offer_callback(CHARSET, lambda: offer_called.append(True) or ["NOPE"]) + wc.set_ext_send_callback(CHARSET, lambda offered: "UTF-8") + + sep = b" " + buf = collections.deque([CHARSET, REQUEST, sep, b"UTF-8"]) + wc._handle_sb_charset(buf) + + assert not offer_called + assert wc.environ_encoding == "UTF-8" + + +def test_server_handle_sb_charset_request_uses_send_callback(): + """Server receiving SB CHARSET REQUEST from client uses send callback.""" + ws, ts, _ = new_writer(server=True) + ws.remote_option[CHARSET] = True + ws.local_option[CHARSET] = True + + ws.set_ext_offer_callback(CHARSET, lambda: ["SHOULD-NOT-USE"]) + ws.set_ext_send_callback(CHARSET, lambda offered: "CP437" if "CP437" in offered else "") + + sep = b" " + buf = collections.deque([CHARSET, REQUEST, sep, b"CP437"]) + ws._handle_sb_charset(buf) + + assert ws.environ_encoding == "CP437" diff --git a/telnetlib3/tests/test_mccp.py b/telnetlib3/tests/test_mccp.py index 1a8f776..be1fb7b 100644 --- a/telnetlib3/tests/test_mccp.py +++ b/telnetlib3/tests/test_mccp.py @@ -141,7 +141,7 @@ def test_sb_mccp2_sets_activated_flag(self): def test_sb_mccp2_calls_ext_callback(self): w, _t, _p = new_writer(server=False, client=True) received = [] - w.set_ext_callback(MCCP2_COMPRESS, lambda val: received.append(val)) + w.set_ext_callback(MCCP2_COMPRESS, received.append) w.pending_option[SB + MCCP2_COMPRESS] = True buf = collections.deque([MCCP2_COMPRESS]) w.handle_subnegotiation(buf) @@ -358,7 +358,7 @@ async def test_client_corrupt_mccp2_drops_data(self): # Decompressor should be disabled, corrupt data not fed to reader assert client._mccp2_decompressor is None - assert received == [] + assert not received async def test_server_corrupt_mccp3_drops_data(self): """Corrupt MCCP3 data is discarded, not fed to IAC parser.""" @@ -379,7 +379,7 @@ async def test_server_corrupt_mccp3_drops_data(self): server.data_received(b"\x00\x01\x02\x03\xff\xfe\xfd") assert server._mccp3_decompressor is None - assert received == [] + assert not received @pytest.mark.asyncio @@ -500,7 +500,7 @@ async def test_mccp3_end_skips_write_when_closing(self): assert client._mccp3_compressor is None # No final flush written because transport is closing - assert transport.writes == [] + assert not transport.writes async def test_mccp3_end_noop_when_inactive(self): """_mccp3_end is safe to call when compression is not active.""" diff --git a/telnetlib3/tests/test_stream_writer_full.py b/telnetlib3/tests/test_stream_writer_full.py index 4909925..9c26b42 100644 --- a/telnetlib3/tests/test_stream_writer_full.py +++ b/telnetlib3/tests/test_stream_writer_full.py @@ -561,7 +561,7 @@ def test_request_tspeed_and_charset_pending_branches(): assert w.request_tspeed() is False w.local_option[CHARSET] = True - w.set_ext_send_callback(CHARSET, lambda: ["UTF-8"]) + w.set_ext_offer_callback(CHARSET, lambda: ["UTF-8"]) assert w.request_charset() is True assert w.request_charset() is False @@ -569,7 +569,7 @@ def test_request_tspeed_and_charset_pending_branches(): def test_request_environ_pending_branch(): w, t, p = new_writer(server=True) w.remote_option[NEW_ENVIRON] = True - w.set_ext_send_callback(NEW_ENVIRON, lambda: ["USER"]) + w.set_ext_offer_callback(NEW_ENVIRON, lambda: ["USER"]) # mark request as pending w.pending_option[SB + NEW_ENVIRON] = True assert w.request_environ() is False @@ -1326,21 +1326,33 @@ def test_request_environ_server_side_conditions(): assert ws.request_environ() is False ws.remote_option[NEW_ENVIRON] = True - ws.set_ext_send_callback(NEW_ENVIRON, lambda: []) + ws.set_ext_offer_callback(NEW_ENVIRON, lambda: []) assert ws.request_environ() is False - ws.set_ext_send_callback(NEW_ENVIRON, lambda: ["USER", "LANG"]) + ws.set_ext_offer_callback(NEW_ENVIRON, lambda: ["USER", "LANG"]) assert ws.request_environ() is True frame = ts.writes[-1] assert frame.startswith(IAC + SB + NEW_ENVIRON + SEND) assert frame.endswith(IAC + SE) +def test_request_environ_uses_offer_not_send_callback(): + ws, ts, _ = new_writer(server=True) + ws.remote_option[NEW_ENVIRON] = True + + send_called = [] + ws.set_ext_offer_callback(NEW_ENVIRON, lambda: ["USER"]) + ws.set_ext_send_callback(NEW_ENVIRON, lambda keys: send_called.append(keys) or {}) + + assert ws.request_environ() is True + assert not send_called + + def test_request_charset_and_xdisploc_and_ttype(): ws, ts, _ = new_writer(server=True) assert ws.request_charset() is False ws.remote_option[CHARSET] = True - ws.set_ext_send_callback(CHARSET, lambda: ["UTF-8", "ASCII"]) + ws.set_ext_offer_callback(CHARSET, lambda: ["UTF-8", "ASCII"]) assert ws.request_charset() is True assert ts.writes[-1].startswith(IAC + SB + CHARSET + b"\x01")