Fix reference link title incorrectly kept when followed by trailing garbage - #627
Open
vjymisal0 wants to merge 1 commit into
Open
Fix reference link title incorrectly kept when followed by trailing garbage#627vjymisal0 wants to merge 1 commit into
vjymisal0 wants to merge 1 commit into
Conversation
cmark_parse_reference_inline() speculatively parses a link title after
a reference definition's URL. If what follows the title is not the end
of the line (i.e. there is trailing garbage), the function rewinds and
retries treating the URL's line as ending right after the URL, so that
the "title" text is left as ordinary paragraph content instead of
being consumed as part of the reference definition.
However, on this rewind path the previously scanned `title` chunk was
never reset, so the reference was still registered with that title
even though it was determined to be invalid. This let garbage-suffixed
"titles" leak into the parsed link, e.g.:
[bla]: /bla
"The title" garbage
[bla]
incorrectly produced `<a href="/bla" title="The title">`, even though
the CommonMark reference dingus (and any spec-conformant parser)
produces no title here, since `"The title" garbage` is not a valid
title specification.
Fix: reset `title` to an empty chunk on the rewind path, matching the
behavior of the "no title matched" branch just below it.
Added a regression test (Issue commonmark#468) to test/regression.txt covering
this exact case.
Fixes commonmark#468
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.
What
Fixes #468.
cmark_parse_reference_inline()insrc/inlines.cspeculatively parses a link title following a reference definition's URL. When the text after the parsed title isn't the end of the line (i.e. there's trailing garbage after the closing quote), the function correctly rewinds and re-checks whether the definition can still be valid by treating the URL's line as ending right after the URL — leaving the "title" text to be parsed as ordinary paragraph content on the next line.The bug: on that rewind path, the
titlechunk that had already been speculatively parsed was never reset. So even though the code determined the title was invalid (it decided the reference has no title, and left"The title" garbageas plain text of the next paragraph), it still passed the stale, invalidtitlechunk intocmark_reference_create().Example:
Before this fix:
The CommonMark reference dingus (and this same cmark binary via
cmark --version's own paragraph output for the discarded title text) shows there should be no title attribute here, since"The title" garbageis not a valid title specification — only"The title"on its own would be.After this fix:
Fix
Reset
titleto an empty chunk (cmark_chunk_literal("")) on the rewind branch, mirroring what the "title didn't match at all" branch already does a few lines below.Testing
masterwith the CLI (cmark.exebuilt via CMake/MinGW), confirmed the fix resolves it, and confirmed normal (non-garbage-suffixed) titles — both single-line[bla]: /bla "The title"and title-on-next-line — still parse correctly.test/regression.txt(Issue #468) covering this exact case.ctest -R spectest_executable) — all pass.ctest -R regressiontest_executable— all 27 regression cases pass, including the new one.ctest -R html_normalizationandctest -R smartpuncttest_executablealso pass.api_testfails in my local Windows/MinGW build with a DLL-load exception (0xc0000135) both before and after this change (verified by stashing the fix and rebuilding), so it's a pre-existing local toolchain issue unrelated to this diff, not a regression.