perf(rust): remove redundant copies and re-parsing on secondary paths (#125) - #131
Merged
Merged
Conversation
Several secondary decode/encode paths did work more than once for no
reason:
- get_unverified_header copied the token into an owned String before
py.detach even though py.detach only requires the closure to be
Ungil (Send), not 'static -- a borrowed &str already satisfies that.
It also ran its own split_compact_segments pre-check right before
parse_compact_header_json ran the exact same split internally.
- decode_unverified used jsonwebtoken::dangerous::insecure_decode,
which fully deserializes the header into jsonwebtoken's typed
Header struct even though only .claims is ever read, and
re-implements its own lenient segment split that silently misparses
a token with extra '.'s instead of rejecting it (unlike our own
split_compact_segments) -- on top of the same redundant pre-check
as get_unverified_header. Replaced with a new single-pass
jws::parse_compact_claims_unverified that reuses the strict split,
parses the header only far enough to confirm it's a JSON object
(matching get_unverified_header's own check), then discards it, and
parses only the payload.
- decode_verified_complete decoded the signature segment's base64 a
second time via a separate extract_signature_bytes call, which
re-split the *entire* token from scratch just to reach that one
segment. verify_and_parse is now verify_and_parse_impl with an
optional with_signature flag: when set, it decodes the signature
once, inline, right where the token is already split for
verification. jsonwebtoken's crypto::verify still does its own
internal base64 decode of the (small, bounded) signature segment,
since it has no public entry point that accepts pre-decoded bytes --
removing the redundant full token re-split was the point, not the
second signature-segment decode alone. extract_signature_bytes is
now unused and removed.
- jws_parse_compact (decode_complete's unverified path) computed and
returned a header.payload "signing input" byte string that its only
Python caller (api_jwt.py) immediately discarded. It no longer
computes it at all; parse_compact_jws's return type drops from a
4-tuple to a 3-tuple (header, payload, signature).
- encode_json (and the RSA fast path it shares with encode via
sign_compact_with_cached_rsa) built the final token through a chain
of Engine::encode calls into throwaway Strings plus two format!s,
each copying everything built so far into a new allocation. Both
now encode header and payload directly into one pre-sized String
(signing_input_string) and append the signature to the same buffer.
No behavioural change, except one narrow, previously-untested edge
case: decode_unverified now accepts a header that is valid JSON but
not a recognized alg name (e.g. {"alg": "made-up"}), since it no
longer deserializes into jsonwebtoken's typed Header struct. This
aligns it with get_unverified_header, which already only required the
header to be a JSON object; no other unverified-decode method in this
library enforces alg recognition, and unverified decode was never a
security boundary.
Measured (release build, HS256, small payload): get_unverified_header
~384ns -> ~352ns; decode_unverified ~681ns -> ~595ns; encode_json
~787ns -> ~666ns; decode_complete(verify_signature=False) end-to-end
~2580ns -> ~2500ns (mostly Python-side claim validation, so the native
saving is a smaller share of the total there).
Closes #125
Regression coverage for decode_unverified's move to a single-pass claims-only parser: the header must still be rejected when it isn't a JSON object at all, and must now be accepted (matching get_unverified_header) when it's a well-formed JSON object with an alg name jsonwebtoken's typed Header struct wouldn't recognize -- pinning the one intentional, narrow behaviour change from the previous commit.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Several small redundant allocations/re-parses on secondary decode/encode paths, all flagged in #125:
get_unverified_header/decode_unverified:token.to_owned()just to move intodetach— a&strcan be captured directly.get_unverified_headeralso split the token twice.decode_unverifieduseddangerous::insecure_decode, which parses the header more than once (intojsonwebtoken's typedHeader, even though only claims are read) and re-implements its own lenient segment split.decode_verified_complete: token split again and the signature base64-decoded twice.jws_parse_compactcopied the token and returned asigning_inputbytes object the only Python caller discards.encode_json:payload_bytes.to_vec()plus twoformat!allocations.Fix (each item addressed)
get_unverified_header: dropped thetoken.to_owned()(py.detachonly requiresUngil/Send, not'static— a&stralready qualifies) and the redundantensure_valid_compact_jwtpre-split (parse_compact_header_jsonalready runs the identical split internally).decode_unverified: replacedjsonwebtoken::dangerous::insecure_decodewith a newjws::parse_compact_claims_unverified— one strict split (our size cap + segment-count check, unlikeinsecure_decode's own lenient one that silently misparses extra.s instead of rejecting them), a JSON-object check on the header (matchingget_unverified_header's own requirement) with the value discarded, and a payload parse. Sameto_owned()removal as above.decode_verified_complete:verify_and_parseis nowverify_and_parse_implwith an optionalwith_signatureflag — when set, it decodes the signature segment inline, right where the token is already split for verification, instead of a separateextract_signature_bytescall that re-split the entire token from scratch to reach it.extract_signature_bytesis now dead and removed.jws_parse_compact: no longer computes the unusedsigning_input;parse_compact_jws's return type drops from a 4-tuple to a 3-tuple. Updated the Python caller and.pyistub.encode_json(and the RSA fast path it shares withencodeviasign_compact_with_cached_rsa): both now build the token through one pre-sizedString(signing_input_string, plus appending the signature) instead of a chain ofEngine::encodecalls into throwawayStrings and twoformat!s.Behavior
No behavioral change, with one narrow, previously-untested exception:
decode_unverifiednow accepts a header that's valid JSON but not a recognizedalgname (e.g.{"alg": "made-up"}), since it no longer deserializes intojsonwebtoken's typedHeaderstruct. This aligns it withget_unverified_header, which already only required the header to be a JSON object; no other unverified-decode method in this library enforcesalgrecognition, and unverified decode was never a security boundary. Pinned with a test either way (rejects non-JSON header, accepts unrecognizedalg).Results (release build, HS256, small payload)
get_unverified_headerdecode_unverifiedencode_jsondecode_complete(verify_signature=False)end-to-endThe last one is dominated by Python-side claim validation, so the native saving (from the
jws_parse_compactchange) is a smaller share of the total there — reported honestly rather than only showing the flattering numbers.Checklist
jsonwebtoken::crypto::verifyhas no public entry point accepting pre-decoded signature bytes)cargo build/clippy --all-targets -D warnings/fmt --check/cargo testclean with bothaws_lc_rs(default) and--no-default-features --features rust_cryptopytest(273 passed, 1 skipped) andmypycleanCloses #125.