fix(make-pdf): stop URLs from swallowing smartypants placeholders#2280
Open
marshaung wants to merge 1 commit into
Open
fix(make-pdf): stop URLs from swallowing smartypants placeholders#2280marshaung wants to merge 1 commit into
marshaung wants to merge 1 commit into
Conversation
URL_RE (/\bhttps?:\/\/\S+/g) is carved after TAG_RE, but the placeholder TAG_RE leaves behind (\u0000SMARTPANTS_PRESERVED_N\u0000) contains no whitespace. A URL sitting flush against a tag therefore matches the URL plus the following placeholder: <p>see <a href="https://ex.com">https://ex.com</a> ok</p> carves to `...PH_1https://ex.comPH_2 ok...`, and \S+ grabs `https://ex.comPH_2`. Restore is a single pass, so the nested PH_2 never restores: the literal text SMARTPANTS_PRESERVED_2 renders in the PDF and the `</a>` is gone, leaving an unclosed anchor that bleeds link-blue through every following paragraph until some later `</a>` closes it. A bold URL swallows `</a></strong>` and leaks bold too. Exclude the sentinel from URL_RE so carved placeholders survive. The existing "does NOT touch URLs" test missed this because its URL is followed by a space, not a tag. Fixes garrytan#2084 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
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.
Fixes #2084.
Root cause
make-pdf/src/smartypants.tscarves preserved zones in this order:TAG_REreplaces each tag with a placeholder (\u0000SMARTPANTS_PRESERVED_N\u0000),which contains no whitespace.
URL_REis/\bhttps?:\/\/\S+/g, so when aURL sits flush against a tag,
\S+runs straight through the placeholder andswallows it.
For a bare autolinked URL, marked emits:
which carves to
PH_0see PH_1https://ex.comPH_2 okPH_3.URL_REthen matcheshttps://ex.comPH_2and preserves that as a single entry. Restoration is asingle
.replace()pass, so the nestedPH_2inside the restored text isnever re-scanned:
SMARTPANTS_PRESERVED_2renders into the PDF, and</a>it stood for is gone — an unclosed anchor that bleedslink-blue through every following paragraph until some later
</a>happensto close it.
**bold URL**swallows</a></strong>and leaks bold as well.[text](https://…)is fine: the URL only lives inside an attribute, whichTAG_REalready carved as part of the whole tag.Fix
Exclude the sentinel from
URL_REso carved placeholders survive:Tests
Three regression tests added to
make-pdf/test/render.test.ts(bare URL, boldURL, URL followed by punctuation). Verified they are load-bearing:
smartypants.tsalone → 3 failrender.test.ts), 188 pass / 0 fail(full
make-pdf/test/, 9 e2e skipped — no built binary in a fresh clone)The existing
does NOT touch URLstest missed this because its URL is followedby a space rather than a tag.
Repro before the fix
pdftotexton a PDF generated from a bare URL, using the shipped 1.58.5.0binary:
with everything after that paragraph rendered blue.