[pull] master from jquast:master

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

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"

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]:

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.

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)")
 

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)

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,

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.

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"

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."""