Refuse the type at the coercion, not three modules later (#776)

CHANGELOG.md

@@ -2001,6 +2001,70 @@ documented at release-notes length in the first place, and are still in
 
 ### The public API and the module layout
 
+- **Five coercions trusted their annotation** (issue #776), and the
+  input-validation gate held fifty-nine public functions open on them.
+  Each takes "anything convertible", handles the `str` spelling and
+  passed everything else through untouched, so the value failed later
+  and somewhere else: a native `TypeError` or `AttributeError` about a
+  builtin, out of a module the caller never called. It is the bug issue
+  #744 fixed in `bytes_from_octets`, five times over.
+
+  `base58.decode` reached `len(v)` with whatever it was handed, and
+  every key and address converter in the library decodes there, which is
+  why one line accounted for more than half the list -- `b58.p2pkh`,
+  `b32.p2wpkh`, `bip32.derive`, all of `bip85` and all of `slip132`,
+  both `to_prv_key` converters and all four of `to_pub_key`'s among
+  them. `bip32.der_path`'s `_indexes_from_der_path` handed a float to
+  `list()`, which answers "'float' object is not iterable";
+  `b32.has_segwit_prefix` assumed bytes in the `else` of its one line;
+  `curves.curve_group.is_on_curve` asked `len` of what is not sized.
+
+  `utils.bytesio_from_binarydata` did not raise at all: it returned its
+  argument unchanged, so a `None` came back a `None` and the six `parse`
+  functions above it failed on `.read` or on `.getbuffer`. It wraps
+  octets in a `BytesIO` now and hands a `BytesIO` back as it came, what
+  is neither being `bytes_from_octets`'s to refuse. A file object is
+  neither, and was never more than half accepted: it has no
+  `getbuffer()`, which `psbt.psbt_utils.deserialize_map` asks the result
+  for. `read_exactly` is still the one that takes any `BinaryIO`, and
+  `psbt.psbt_view` still reads a psbt out of an open file through it.
+
+  `utils.str_from_string` is the `String` half of `bytes_from_octets`,
+  and the four places that spelled that coercion out by hand now call
+  it: `bech32._decode`, `b32.has_segwit_prefix`,
+  `b32.witness_from_address` and `silent_payments.keys_from_address`. It
+  takes text or ascii bytes and refuses the rest, which is what the two
+  address functions needed *before* their length bound -- `len` of a
+  float is a complaint about a builtin, where the codec below would have
+  named the argument -- and it carries to all four the non-ascii refusal
+  that only `bech32` had, a `UnicodeDecodeError` being outside the
+  contract a caller is told to catch.
+
+  `script.script_pub_key.address` answered `""` for a `None`, which is
+  the answer a nulldata output has: the coercion runs before the
+  truthiness now, so an argument that is no script is refused rather
+  than reported as a script that has no address. `tx_or_psbt_from_any`
+  and `script.taproot.serialize` are the last two, one reaching `.split`
+  and the other subscripting what it was given.
+
+  **What moves for a caller**: the class is narrower and the control
+  flow identical, `BTClibTypeError` being a `TypeError` and
+  `BTClibValueError` a `ValueError`. `to_prv_key`'s WIF and xkey
+  attempts catch a `TypeError` beside the `ValueError` now, as its two
+  "it must be octets" fallbacks already did, so an argument of the wrong
+  type still ends as "not a private key" and not as the first format's
+  refusal. A `bytearray` or a `memoryview` is accepted wherever a
+  `String` is, `base58.decode` and `str_from_string` taking the buffers
+  `bytes_from_octets` takes.
+
+  What the gate still holds open is one function, and it is not a
+  coercion: `ecc.dleq.verify_proof` answers False for a pub key that is
+  no point, `to_pub_key` refusing one as a `BTClibValueError` whatever
+  was wrong with it. That is what keeps a boolean verification total,
+  which is issue #745's decision and issue #143's test -- `dsa.verify`
+  answers False for a private key passed as a public one -- so closing
+  that entry is reversing those rather than plugging a hole.
+
 - **`bytes_from_octets` is the whole of what `Octets` means, and it is
   total** (issue #744). It is the coercion every `Octets` parameter of
   the library runs through, and it had two holes: a hex string that is
@@ -3040,6 +3104,16 @@ documented at release-notes length in the first place, and are still in
 
 ### Tests
 
+- **The input-validation gate's own verdict is exercised** (issue #776).
+  `_classify` is the three answers a call can give -- the rule held, a
+  native exception got out and here is its class, nothing was raised at
+  all -- and it is a function of its own now rather than three branches
+  inside the walk. The middle one is why: no function under `btclib/`
+  produces a native exception any more, so as a branch it went uncovered,
+  and an unrun branch is a poor thing to be relying on the day one does.
+  `test_the_walk_names_what_escapes` provokes all three on calls whose
+  behaviour is stated there.
+
 - **Every test that reads a source file names its encoding.** Four
   `read_text()` calls took the locale's, which is UTF-8 on the runners this
   suite is usually read on and cp1252 on the Windows ones: the walk in

btclib/alias.py

@@ -8,9 +8,9 @@
 one from the other: passing a text string where a hex-string is expected
 is a type error this file names but no checker can catch. The distinction
 is enforced at run time instead, by the converter each function calls on
-its way in -- bytes_from_octets for Octets, encode() for String -- and it
-is documented here because that is the only place it can be read as one
-piece.
+its way in -- bytes_from_octets for Octets, str_from_string for String --
+and it is documented here because that is the only place it can be read
+as one piece.
 
 Making them NewTypes would let mypy separate them, at the cost of every
 caller having to wrap its literals: Octets("deadbeef") instead of

btclib/b32.py

@@ -60,7 +60,7 @@
 from btclib.hashes import hash160, sha256
 from btclib.network import NETWORKS, network_from_key_value, network_from_name
 from btclib.to_pub_key import Key, pub_keyinfo_from_key
-from btclib.utils import bytes_from_octets
+from btclib.utils import bytes_from_octets, str_from_string
 
 __all__ = [
     "address_from_witness",
@@ -82,7 +82,7 @@ def has_segwit_prefix(addr: String) -> bool:
     The prefix alone -- hrp and the 1 separator -- is read; whether the
     rest decodes is witness_from_address's answer.
     """
-    str_addr = addr.strip().lower() if isinstance(addr, str) else addr.decode("ascii")
+    str_addr = str_from_string(addr, "address").strip().lower()
     return any(str_addr.startswith(f"{net.hrp}1") for net in NETWORKS.values())
 
 
@@ -161,15 +161,17 @@ def witness_from_address(b32addr: String) -> tuple[int, bytes, str]:
 
     The returned data structure is: version, program, network.
     """
-    if isinstance(b32addr, str):
-        b32addr = b32addr.strip()
+    # the coercion before the length, which is a fact about characters:
+    # `len` of what is neither text nor bytes is a TypeError about a
+    # builtin, where the codec below would have named the argument
+    addr = str_from_string(b32addr, "address").strip()
 
     # the 90-character bound is address semantics, deliberately not
     # enforced by the bech32 codec (Lightning strings exceed it)
-    if len(b32addr) > 90:
-        raise BTClibValueError(f"invalid bech32 address length: {len(b32addr)} > 90")
+    if len(addr) > 90:
+        raise BTClibValueError(f"invalid bech32 address length: {len(addr)} > 90")
 
-    hrp, data = decode(b32addr)
+    hrp, data = decode(addr)
 
     wit_ver = data[0]
     wit_prog = bytes(power_of_2_base_conversion(data[1:], 5, 8, False))

btclib/base58.py

@@ -154,13 +154,10 @@ def _b58decode_to_int(v: bytes) -> int:
 
 
 def _b58decode(v: bytes) -> bytes:
-    # bytes for what the character-by-character check took without ever
-    # saying so: any iterable of byte values. That is how a caller's
-    # garbage reaches here -- `to_prv_key` tries a WIF before it gives up,
-    # so a Point handed to it arrives as the tuple (5, 0) -- and it has to
-    # go on being refused below as characters outside the alphabet, which
-    # is a BTClibValueError, rather than as a missing method. Free on the
-    # path that matters: bytes(b) is b for a bytes object, 29 ns
+    # bytes for the buffers that are a String and are not bytes: a
+    # memoryview has neither translate nor lstrip, and a bytearray
+    # answers both in its own type. Free on the path that matters:
+    # bytes(b) is b for a bytes object, 29 ns
     v = bytes(v)
     # every alphabet byte deleted, so what is left is what is not one:
     # the same question as `any(x not in _ALPHABET for x in v)` asked in
@@ -202,6 +199,15 @@ def decode(v: String, out_size: int | None = None) -> bytes:
             v = v.encode("ascii")
         except UnicodeEncodeError as e:
             raise BTClibValueError(f"non-ascii character in base58 string: {e}") from e
+    elif not isinstance(v, (bytes, bytearray, memoryview)):
+        # what is neither went through untouched and failed on `len` below,
+        # which is a TypeError about a builtin rather than about the
+        # address that was passed. Every key and address converter in the
+        # library decodes here, so this is the one place worth saying it
+        # in. Every buffer and not `bytes` alone, as bytes_from_octets
+        # takes them
+        err_msg = f"invalid base58 string type: {type(v).__name__}"  # type: ignore[unreachable]
+        raise BTClibTypeError(err_msg)
 
     if len(v) > MAX_LENGTH:
         err_msg = f"too many base58 characters: {len(v)}, max is {MAX_LENGTH}"

btclib/bech32.py

@@ -59,7 +59,7 @@
 
 from btclib.alias import String
 from btclib.exceptions import BTClibTypeError, BTClibValueError
-from btclib.utils import is_integer
+from btclib.utils import is_integer, str_from_string
 
 __all__ = [
     "decode",
@@ -125,43 +125,38 @@ def _verify_checksum(hrp: str, data: list[int], m: int) -> bool:
 
 def _decode(bech: String) -> tuple[str, list[int], list[int]]:
     """Determine a bech32 string HRP, data and checksum."""
-    if isinstance(bech, bytes):
-        # bech32 is an ascii encoding, so a byte outside it is an
-        # invalid character like any other and gets the same answer:
-        # a UnicodeDecodeError let out would fly past every caller
-        # written to catch BTClibValueError
-        try:
-            bech = bech.decode("ascii")
-        except UnicodeDecodeError as e:
-            raise BTClibValueError(f"non-ascii character in bech32 string: {e}") from e
+    # bech32 is an ascii encoding, so a byte outside it is an invalid
+    # character like any other, and what is neither text nor bytes is
+    # refused here rather than reaching `rfind` as a missing method
+    text = str_from_string(bech, "bech32 string")
 
     # no 90-character limit here: that bound belongs to bitcoin
     # addresses and not to bech32, which the Lightning Network uses
     # without it. The deferral is carried out rather than merely
     # intended -- b32.witness_from_address enforces it, and the module
     # docstring there lists it among the rules b32 adds on top
 
-    pos = bech.rfind("1")  # find the separator between hrp and data
+    pos = text.rfind("1")  # find the separator between hrp and data
     if pos == -1:
-        raise BTClibValueError(f"no separator character: {bech}")
+        raise BTClibValueError(f"no separator character: {text}")
     if pos == 0:
-        raise BTClibValueError(f"empty HRP: {bech}")
-    if pos + 7 > len(bech):
-        raise BTClibValueError(f"too short checksum: {bech}")
+        raise BTClibValueError(f"empty HRP: {text}")
+    if pos + 7 > len(text):
+        raise BTClibValueError(f"too short checksum: {text}")
 
-    if not all(47 < ord(x) < 123 for x in bech[:pos]):
-        raise BTClibValueError(f"HRP character out of range: {bech}")
-    if bech.lower() != bech and bech.upper() != bech:
-        raise BTClibValueError(f"mixed case: {bech}")
+    if not all(47 < ord(x) < 123 for x in text[:pos]):
+        raise BTClibValueError(f"HRP character out of range: {text}")
+    if text.lower() != text and text.upper() != text:
+        raise BTClibValueError(f"mixed case: {text}")
 
-    bech = bech.lower()
-    hrp = bech[:pos]
+    text = text.lower()
+    hrp = text[:pos]
 
-    indices = [_INDEX_OF.get(x, -1) for x in bech[pos + 1 :]]
+    indices = [_INDEX_OF.get(x, -1) for x in text[pos + 1 :]]
     if -1 in indices[-6:]:
-        raise BTClibValueError(f"invalid character in checksum: {bech}")
+        raise BTClibValueError(f"invalid character in checksum: {text}")
     if -1 in indices:
-        raise BTClibValueError(f"invalid data character: {bech}")
+        raise BTClibValueError(f"invalid data character: {text}")
     data = indices
 
     return hrp, data[:-6], data[-6:]

btclib/bip32/der_path.py

@@ -21,7 +21,7 @@
 from __future__ import annotations
 
 import re
-from collections.abc import Sequence
+from collections.abc import Iterable, Sequence
 
 from btclib.alias import Octets
 from btclib.exceptions import BTClibTypeError, BTClibValueError
@@ -236,6 +236,13 @@ def _indexes_from_der_path(der_path: Sequence[int] | int | bytes) -> list[int]:
             for n in range(0, len(der_path), 4)
         ]
 
+    if not isinstance(der_path, Iterable):
+        # what is left went to `list()` untouched, which answers a float
+        # with "'float' object is not iterable" -- a complaint about
+        # iteration, from underneath the library, about a path
+        err_msg = f"invalid derivation path type: {type(der_path).__name__}"  # type: ignore[unreachable]
+        raise BTClibTypeError(err_msg)
+
     # an iterable of int, and of int alone: int() here would coerce a bool
     # into the index one, where the annotation already says Sequence[int]
     indexes = list(der_path)

btclib/curves/curve_group.py

@@ -528,6 +528,10 @@ def require_on_curve(self, Q: Point) -> None:
 
     def is_on_curve(self, Q: Point) -> bool:
         """Return True if the point is on the curve."""
+        # the type before the length: `len` of what is not sized is a
+        # TypeError about a builtin, where a Point is what this asks for
+        if not isinstance(Q, tuple):
+            raise BTClibTypeError(f"invalid point type: {type(Q).__name__}")
         if len(Q) != 2:
             raise BTClibValueError("point must be a tuple[int, int]")
         if Q[1] == 0:  # Infinity point in affine coordinates

btclib/exceptions.py

@@ -25,8 +25,8 @@
 class; `except BTClibException` is for the caller who only needs to know
 it came from here.
 
-That every failure of btclib's *is* one is not yet true: issue #744
-counts the public functions still letting a native `KeyError`,
+That every failure of btclib's *is* one is not yet true: issue #776
+carries the public functions still letting a native `KeyError`,
 `IndexError` or `OverflowError` through, and until that is closed this
 class catches most of what the library raises rather than all of it.
 

btclib/script/script_pub_key.py

@@ -69,6 +69,12 @@ def address(script_pub_key: Octets, network: str = "mainnet") -> str:
     can read the address back into the very script it came from
     (issue #251).
     """
+    # the coercion before the truthiness, which is what tells an empty
+    # script from a script that has no address: `None` is falsy just as
+    # `b""` is, so a caller passing one was answered "" -- the answer a
+    # nulldata output has, and not a word about the argument
+    script_pub_key = bytes_from_octets(script_pub_key)
+
     if script_pub_key:
         script_type, payload = type_and_payload(script_pub_key)
         if script_type in {"p2pkh", "p2sh"}:
@@ -80,7 +86,7 @@ def address(script_pub_key: Octets, network: str = "mainnet") -> str:
         if script_type == "witness_unknown":
             # the one type whose version the answer does not imply: it is
             # the op code the program follows, OP_2..OP_16 being 0x52..0x60
-            version = bytes_from_octets(script_pub_key)[0] - 0x50
+            version = script_pub_key[0] - 0x50
             return b32.address_from_witness(version, payload, network)
 
     # not script_pub_key

btclib/script/taproot.py

@@ -79,6 +79,12 @@ def serialize(script: ScriptList) -> bytes:
     by exactly one bytes command, appended raw -- what follows an
     OP_SUCCESS need not be a script, so it round-trips unparsed.
     """
+    if not isinstance(script, list):
+        # what is not a list reached the reversal and the `pop` below
+        # untouched, so a None was "not subscriptable" and a str was a
+        # str with no `pop` -- neither of them a word about the script
+        raise BTClibTypeError(f"invalid tapscript type: {type(script).__name__}")
+
     r: list[bytes] = []
     script = script[::-1]
     while script:

btclib/silent_payments.py

@@ -77,7 +77,7 @@
 from btclib.to_prv_key import PrvKey, int_from_prv_key
 from btclib.to_pub_key import PubKey, point_from_pub_key
 from btclib.tx.out_point import OutPoint
-from btclib.utils import bytes_from_octets, is_integer
+from btclib.utils import bytes_from_octets, is_integer, str_from_string
 
 __all__ = [
     "K_MAX",
@@ -247,13 +247,15 @@ def keys_from_address(address: String) -> tuple[Point, Point, NetworkType]:
     pay a later address. v31 is refused instead, being the version BIP352
     reserves for a change that breaks exactly that.
     """
-    if isinstance(address, str):
-        address = address.strip().lower()
-    if len(address) > _MAX_ADDRESS_SIZE:
-        err_msg = f"invalid address length: {len(address)} > {_MAX_ADDRESS_SIZE}"
+    # the coercion before the length, as in b32.witness_from_address:
+    # `len` of what is neither text nor bytes is a TypeError about a
+    # builtin, where the codec below would have named the argument
+    addr = str_from_string(address, "address").strip().lower()
+    if len(addr) > _MAX_ADDRESS_SIZE:
+        err_msg = f"invalid address length: {len(addr)} > {_MAX_ADDRESS_SIZE}"
         raise BTClibValueError(err_msg)
 
-    hrp, data = decode(address, _BECH32_M_CONST)
+    hrp, data = decode(addr, _BECH32_M_CONST)
     if hrp == _MAINNET_HRP:
         network_type: NetworkType = "main"
     elif hrp == _TESTNET_HRP:

btclib/to_prv_key.py

@@ -188,13 +188,13 @@ def _prv_keyinfo_from_wif(
     # echoes the input, which is candidate key material -- a checksum, a
     # prefix and a size are not secret.
     #
-    # ValueError and not BTClibValueError: b58decode leaks a plain one,
-    # "byte must be in range(0, 256)", for an input that is neither str nor
-    # bytes -- a Point tuple, say. That leak is base58's to fix; whatever
-    # it is, it means this input is not a WIF
+    # both classes, as the octets branch of the two public converters
+    # catches both: what is neither a base58 string nor bytes is a
+    # TypeError, and it means here what a bad checksum means -- this input
+    # is not a WIF, and the caller is free to try it as something else
     try:
         payload = b58decode(wif)
-    except ValueError as e:
+    except (TypeError, ValueError) as e:
         raise NotAPrvKeyError(f"not a WIF ({e})") from e
 
     # from here on the version prefix says WIF, so a fault in what follows
@@ -226,10 +226,13 @@ def _prv_keyinfo_from_xprv(
     else:
         # base58, 78 bytes, and a known xkey version or not: a negative
         # answer leaves the input free to be octets or an int, so it is
-        # NotAPrvKeyError, carrying the reason rather than discarding it
+        # NotAPrvKeyError, carrying the reason rather than discarding it.
+        # Both classes, as everywhere a format is guessed here: a type the
+        # decode refuses says this is not an xkey, not that the caller is
+        # owed a TypeError from inside the guessing
         try:
             xprv = _key_data_from_bip32_key(xprv)
-        except ValueError as e:
+        except (TypeError, ValueError) as e:
             raise NotAPrvKeyError(f"not a BIP32 xkey ({e})") from e
 
     # a BIP32 key is always compressed, so a caller asking for uncompressed