Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix edge cases for relaxed-autolink option #461

Merged
merged 2 commits into from
Aug 31, 2024
Merged

Conversation

digitalmoksha
Copy link
Collaborator

@digitalmoksha digitalmoksha commented Aug 26, 2024

Handle several edge cases with the relaxed-autolinks option when autolinking inside a markdown link. We now detect if it's a single url in a markdown link ([url](url)). We also handle if an autolink does get added to a link by removing the duplicate link when converting to HTML.

Related issue: #459

Copy link
Contributor

Command Mean [ms] Min [ms] Max [ms] Relative
./bench.sh ./comrak-aef95c6 323.2 ± 1.3 321.4 327.3 2.92 ± 0.02
./bench.sh ./comrak-main 326.6 ± 1.3 325.0 331.5 2.95 ± 0.02
./bench.sh ./pulldown-cmark 110.8 ± 0.5 109.8 111.8 1.00
./bench.sh ./cmark-gfm 118.9 ± 1.3 117.5 122.5 1.07 ± 0.01
./bench.sh ./markdown-it 480.7 ± 4.4 476.2 493.0 4.34 ± 0.04

Run on Mon Aug 26 18:55:08 UTC 2024

@digitalmoksha digitalmoksha force-pushed the bw-fix-relaxed-autolinks branch 3 times, most recently from b08a0a8 to ee23b32 Compare August 26, 2024 19:32
@kivikakk
Copy link
Owner

This fails fuzzing. A minimised all_options test case is here: minimized-from-7305067d8bb96107d864bbcb270f09d57ab62a42.md. Repro with cargo fuzz run all_options minimized-from-7305067d8bb96107d864bbcb270f09d57ab62a42.md.

I assume i + link_end may be zero.

when using the `relaxed-autolinks` option.

See #459
@digitalmoksha digitalmoksha force-pushed the bw-fix-relaxed-autolinks branch from ee23b32 to e982dbd Compare August 30, 2024 21:40
@digitalmoksha
Copy link
Collaborator Author

Thanks @kivikakk, that does seem to be it. I've pushed a change, and am running cargo +nightly fuzz run all_options -j 6

@kivikakk
Copy link
Owner

LGTM — thanks!

@kivikakk kivikakk enabled auto-merge August 31, 2024 08:30
@kivikakk kivikakk merged commit 92efde0 into main Aug 31, 2024
39 checks passed
@kivikakk kivikakk deleted the bw-fix-relaxed-autolinks branch August 31, 2024 08:33
@digitalmoksha
Copy link
Collaborator Author

@kivikakk are you up for publishing a new release?

@kivikakk
Copy link
Owner

kivikakk commented Sep 5, 2024

Sure thing; will do so in the next hour or so.

@kivikakk
Copy link
Owner

kivikakk commented Sep 5, 2024

It's on crates.io :).

@digitalmoksha
Copy link
Collaborator Author

Thank you! 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants