search

mity / md4c

C Markdown parser. Fast. SAX-like interface. Compliant to CommonMark specification.
MIT License
787 stars 146 forks source link

Wiki links #92

Closed niblo closed 4 years ago

niblo commented 5 years ago

This does not feature link labels, only the link target. Comments appreciated.

codecov[bot] commented 5 years ago

Codecov Report

Merging #92 into master will increase coverage by 0.24%. The diff coverage is 98.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
+ Coverage   94.01%   94.26%   +0.24%     
==========================================
  Files           1        1              
  Lines        2642     2722      +80     
==========================================
+ Hits         2484     2566      +82     
+ Misses        158      156       -2
Impacted Files Coverage Δ
md4c/md4c.c 94.26% <98.86%> (+0.24%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ef85cfc...02b773c. Read the comment docs.

mity commented 5 years ago

Seems good to me now so far. Do you plan to continue to work in the foreseeable future in this PR?

(It would be simpler for me, as such a feature deserves proper testing, including some fuzzing over-night so I would wait with that until you consider it complete to do it just once.)

niblo commented 5 years ago

Seems good to me now so far. Do you plan to continue to work in the foreseeable future in this PR?

I will continue with adding support for the label part of the wiki link during the week. The goal is to complete the wiki link feature, with proper support for labels.

I plan to get back to this in about 20 hours or so. In the meantime, thank you for the support.

mity commented 5 years ago

Ok. I would then fuzz-test it together when it is done.

It takes AFL on my machine many hours to cover all the code paths and it is almost unusable for anything else in the mean time. Hopefully will qualify to https://github.com/google/oss-fuzz one day but until then...

mity commented 5 years ago

@niblo Sorry for the long delay. Solving some real-life stuff. I hope to get to it again during the week-end.

niblo commented 5 years ago

Sounds good.

On 19 Sep 2019, at 12:21, Martin Mitáš wrote:

@niblo Sorry for the long delay. Solving some real-life stuff. I hope to get to it again during the week-end.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/mity/md4c/pull/92#issuecomment-533067310

mity commented 4 years ago

Fuzz testing has found many crash(es) in this branch, but likely they are all exhibitions of the same bug (multiple randomly chosen crash with the same backtrace).

Looks to me that there its some interference of code for tables and for wiki-links as it crashes only with --ftables. It even does not need --fwiki-links but the crash does not happen on master branch.

I'll try to minimize and textize the crashing input (it has tens of kB of ugly binary stuff) before posting it here.

mity commented 4 years ago

Minimized one of the crash inputs into this:

[f]: &#ffff;

x|x
---|---
[foo|x]

x|x
---|---
[foo|x]
niblo commented 4 years ago

Thanks for the feedback. I will try to have a look at it this weekend.

mity commented 4 years ago

I have just experimentally merged this PR and PR #97 and indeed, it seems to fix the crash. So, I'll fuzz-test that over the night and if there is no other surprise I'll then merge both tomorrow.

mity commented 4 years ago

Finally merged.

@niblo, thanks for all the work and also for your patience.

niblo commented 4 years ago

It was my pleasure. I would like to thank you too for the very same reasons. Your support made it possible.