MaglMarkdown icon indicating copy to clipboard operation
MaglMarkdown copied to clipboard

erusev/parsedown included in roave/security-advisories

Open steffendietz opened this issue 7 years ago • 4 comments

Due to recent events, the erusev/parsedown package was added to the FriendsOfPHP security advisories for versions <=1.6.4.

https://github.com/FriendsOfPHP/security-advisories/commit/65b70c4a9a05e536522e5b448832d7a389f89e01

This was subsequently picked up by the roave/security-advisories package.

https://github.com/Roave/SecurityAdvisories/commit/f3e52bf601a4224e2eb3ac8e833c129beff8abc8

Since maglnet/magl-markdown is currently requiring the parsedown package, its conflicting with roave/security-advisories. I'm opening this issue here to kind of discuss what a course of action could look like, if any is needed. 😄

steffendietz avatar Feb 26 '18 09:02 steffendietz

Oh, I see, that's bad but not directly a problem with this package, so we can only work around this issue.

I also don't like it that this package requires all different parsers and only uses one of them.

One possibility would be to release a new major and only suggest packages instead of installing them and only provide some sort of dummy-parser that is shipped per default.

Pro:

  • problems like this could be solved
  • we only install the parser we need

Con:

  • users have to require an additional package and configure it before any markdown will be correctly parsed

What do you think?

maglnet avatar Feb 27 '18 09:02 maglnet

That was actually exactly the direction I was aiming for with this issue.

But maybe to avoid the "con" from above, you could determine a default markdown parser this package should use, and suggest the other supported ones.

Of course, if the default package would happen to be the erusev/parsedown regardless, the current situation would still be unchanged, but as you already said, it is no problem with this package.

One could also try to make the argument, that this package provides a common interface and method to integrate markdown libraries into a ZF2 application, but should not decide which library to include as a default. Which would essentially support your suggestion from above.


To answer your closing question: I think, that as soon as a user has to decide, which adapter to use, he could also be bothered to require the matching library in his projects composer.json file. 😄 But I'm probably a bit biased and this might not be the direction you want to go with your package.

steffendietz avatar Feb 27 '18 11:02 steffendietz

Well, then I think removing the direct dependencies and forcing a user to select the renderer is the way I would like to go. I would avoid having a default renderer, as this could cause the same problem again but also makes the initial installation a bit more complicated. I'll dig into this in the next days, when I'm back in the office ;)

maglnet avatar Feb 27 '18 13:02 maglnet

Btw: The issue in parsedown is fixed, so I'll put this on hold, as the installation is possible again and it's not urgent anymore. :-)

maglnet avatar Mar 01 '18 13:03 maglnet