skip protected-header JSON parse when "zip" is absent#186
Merged
Conversation
`handle_zip_enc()` and `zip_in_protected_header()` sit on the JWE encrypt
hot path. For every call they base64url-decode the `protected` header and
run a full JSON parse (`json_loadb`) just to check for a `zip` key.
This gates the expensive JSON parse behind a cheap substring scan: decode the
header, scan the **decoded** bytes for the literal `"zip"`, and only parse when
it's present. When `zip` is absent, we skip the parse and take the existing
no-zip path. A false-positive substring just falls through to the real parse,
so behaviour is unchanged in every case.
An earlier version of this optimization scanned the base64url-**encoded**
`protected` string for `"zip"`. That is incorrect: a `zip` key does not survive
base64url encoding as the literal substring `"zip"` — it lands on shifting
bit boundaries (e.g. `{"...","zip":"DEF"}` encodes with `zip` appearing as
`emlw`). Scanning the encoded form yields false negatives, so compression is
silently skipped on zip-enabled messages and the JWE fails to round-trip. This
version scans the decoded JSON text, where a `zip` key must appear verbatim.
`zip_in_protected_header` on a ~56-byte header, `-O2`, best of 3:
| Header | Before | After | Delta |
|-------------|----------|-----------|------------------|
| `zip` absent | ~4.5 µs | ~3.2 µs | ~30% faster (parse skipped) |
| `zip` present | ~6.0 µs | ~6.0 µs | within noise (+~21 ns memmem) |
The added cost on the (rare) zip-enabled path is a single `memmem` over the
decoded header (~21 ns); the win on the common no-zip path is the eliminated
`json_loadb`.
- `meson test` — **all 26 tests pass**, including `api_jwe`'s zip
compress/decompress round-trip (which the encoded-scan version failed with
SIGABRT).
- Verified the clean base passes the same suite, so no pre-existing failures.
- The pre-check requires `_GNU_SOURCE` for `memmem` and `<jose/cfg.h>` for the
`jose_calloc`/`jose_free` allocators (used to mirror `jose_b64_dec_load`'s
scrub-and-free of the decoded buffer).
- `prt` is a borrowed reference from `json_object_get()` at the early-return
points; it's cleared to `NULL` before returning so the `json_auto_t` cleanup
doesn't spuriously `json_decref` it.
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.
Summary
handle_zip_enc()andzip_in_protected_header()sit on the JWE encrypthot path. For every call they base64url-decode the
protectedheader andrun a full JSON parse (
json_loadb) just to check for azipkey.This gates the expensive JSON parse behind a cheap substring scan: decode the
header, scan the decoded bytes for the literal
"zip", and only parse whenit's present. When
zipis absent, we skip the parse and take the existingno-zip path. A false-positive substring just falls through to the real parse,
so behaviour is unchanged in every case.
Why scan the decoded bytes, not the encoded string
An earlier version of this optimization scanned the base64url-encoded
protectedstring for"zip". That is incorrect: azipkey does not survivebase64url encoding as the literal substring
"zip"— it lands on shiftingbit boundaries (e.g.
{"...","zip":"DEF"}encodes withzipappearing asemlw). Scanning the encoded form yields false negatives, so compression issilently skipped on zip-enabled messages and the JWE fails to round-trip. This
version scans the decoded JSON text, where a
zipkey must appear verbatim.Performance
zip_in_protected_headeron a ~56-byte header,-O2, best of 3:zipabsentzippresentThe added cost on the (rare) zip-enabled path is a single
memmemover thedecoded header (~21 ns); the win on the common no-zip path is the eliminated
json_loadb.Testing
meson test— all 26 tests pass, includingapi_jwe's zipcompress/decompress round-trip (which the encoded-scan version failed with
SIGABRT).
Notes
_GNU_SOURCEformemmemand<jose/cfg.h>for thejose_calloc/jose_freeallocators (used to mirrorjose_b64_dec_load'sscrub-and-free of the decoded buffer).
prtis a borrowed reference fromjson_object_get()at the early-returnpoints; it's cleared to
NULLbefore returning so thejson_auto_tcleanupdoesn't spuriously
json_decrefit.