Orc / discount

My C implementation of John Gruber's Markdown markup language
http://www.pell.portland.or.us/~orc/Code/discount
Other
859 stars 156 forks source link

MKD_SAFELINK should allow relative url fragments #175

Closed jmccl closed 6 years ago

jmccl commented 6 years ago

The comment for MKD_SAFELINK is "paranoid check for link protocol".

The code reads as follows:

    else if ( (f->flags & MKD_SAFELINK) && T(ref->link)
                                        && (T(ref->link)[0] != '/')
                                        && !isautoprefix(T(ref->link), S(ref->link)) )
        /* if MKD_SAFELINK, only accept links that are local or
         * a well-known protocol
         */
        return 0;

If our page displaying markdown is http://foo.com/bar, the following absolute links work.

[link](/baz)   -> https://foo.com/baz
[link](/#baz)  -> https://foo.com/#baz

However this doesn't work, i.e., it just displays the markup.

[link](baz)   
[link](#baz)  

My assertion is that [link](#baz) should be allowed as a link. The '#' character provides as much information as '/' to the browser as to the form of the (local) link.

Orc commented 6 years ago

Huh. Let me dig back through my correspondence about that feature and see if I can find the rationale for excluding /'ed links.

jmccl commented 6 years ago

I'm not sure if my report was clear. The code says links that begin with '/' are 'safe' and links that begin with '#' are not safe. I'm just saying they should be the same.

Thanks for giving this some thought.

Orc commented 6 years ago

It's from when Reddit was using discount (dunno what they use now; I think they rolled their own?) and wanted to discourage vandalism in the form of link

I've not had enough tea this morning, so I can't remember the actual format of a url, let alone completely parse your bug report :-) You're right that fragment 'text#anchor' is exactly the same as '#anchor' when you're coming from 'page' -- it should be fairly easy to allow it, but I don't want to leave an obvious window for pranksters.

Orc commented 6 years ago

Okay, feast your eyes (and your C compiler); I've reworked the safelink test to better check for protocols vs. url fragments. No version# yet, just top of stack.

jmccl commented 6 years ago

Sorry for the delay in responding. My email tracking system failed (as it frequently does.)

Code looks good.

About the only thing I might do is add a comment about what the point of 'safelink' is. I'm assuming it has something to do with the type of thing being discussed here:

http://www.ush.it/team/ascii/URI_Use_and_Abuse.pdf

Until I did some googling I didn't understand the point of it, as 'Surely browsers know how to sensibly interpret a URI?' Apparently that's not the case, but unless you're aware of that the flag's a little baffling. I had turned it on in my code because... I'd rather be safe than not? But I didn't really understand what I was doing.

BTW, nice library. Much more robust than the javascript thing I was using before.

Orc commented 6 years ago

The documentation for discount is erratic; I don't do as well as documenting the code as I should, and a lot of the 3rd party features just got slurped in w/o my working over the documentation to cover it (the bane of free software; coding is easier to do than writing!)