htsparse: fix HTML-escape truncation, cache-buffer leak, and stats-throttle reset - #465
Merged
Conversation
HT_ADD_HTMLESCAPED_ANY reserved strlen*5+1024 on the assumption that "&" (5 bytes) is the worst-case expansion. That holds for escape_for_html_print, but escape_for_html_print_full turns a high byte into "&#xHH;" (6 bytes). Past ~1023 high bytes the reservation is short, so the escaper hits its internal cap: it truncates the string mid-run and its overflow return counts the terminating NUL, which then lands inside the mirrored HTML file. The only _full call site rewrites a link into a 2KB buffer, so a long non-ASCII local path triggers it. Give the macro a per-function expansion factor (HTS_HTMLESCAPE_MAXEXP=5, HTS_HTMLESCAPE_FULL_MAXEXP=6) and pass 6 for the _full variant. A new escape-room self-test pins each function's real worst-case expansion against the constant the macro reserves, so the two can't drift again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The not-modified fast path reads the stored //[HTML-MD5]// digest via cache_readdata, which malloc's the buffer, but never freed it. Every page whose on-disk size already matches the freshly rewritten one leaks that buffer. Free it after the compare. Wrapped the macro in clang-format off/on: it is hand-aligned and clang-format realigns every backslash on any edit, churning untouched lines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
makestat_time throttles the makestat/maketrack stats to once per minute: the wait loop compares time_local() against it and, when it fires, writes it back to the local. But the field is by-value in the extended context, so it can't round-trip through ENGINE_SAVE_CONTEXT, while ENGINE_SET_CONTEXT re-read it from the load-once baseline on every loop iteration. That reset the local before the next compare, so under -%v / maketrack the throttle never held and the stats line plus the full back-stack dump were emitted every iteration. Drop makestat_time (and the never-changing makestat_fp) from SET_CONTEXT; they belong to the load-once set. Wrapped the macro in clang-format off/on for the same backslash-realignment reason as HT_ADD_END. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
xroche
force-pushed
the
htsparse-t0-bugfixes
branch
from
July 1, 2026 08:52
0c36d2b to
b804ee2
Compare
This was referenced Jul 1, 2026
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
1 participant
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.
Three independent bugs in htsparse.c's output-buffer and coroutine-context macros, surfaced by a macro audit.
HT_ADD_HTMLESCAPED*reservedstrlen*5+1024, sized for&, butescape_for_html_print_fullemits 6 bytes (&#xHH;) per high byte. Past ~1023 high bytes the escaper hit its internal cap, truncated the string mid-run, and counted its terminating NUL into the mirrored HTML file. The only_fullcall site rewrites a link into a 2KB buffer, so a long non-ASCII local path reaches it. The macro now takes a per-function expansion factor (6 for_full), pinned by a newescape-roomself-test so the constant cannot drift from what the function emits.HT_ADD_END's not-modified fast path read the stored digest withcache_readdata, which malloc's a buffer, and never freed it: a leak on every page whose on-disk size already matched the rewritten one.makestat_timethrottles the makestat/maketrack stats to once a minute, but it lived inENGINE_SET_CONTEXTand is a by-value context field that cannot round-trip throughENGINE_SAVE_CONTEXT. The wait loop re-read it from the load-once baseline every iteration, resetting the local before the next compare, so under-%v/maketrack the throttle never held. It is load-once now.The two edited macros are hand-aligned; each fix wraps its macro in
clang-format off/onso touching one line does not realign every backslash.