search

maglnet / MaglMarkdown

A ZF2 module that provides a Service and View Helper to support markdown syntax with the ability to easily switch between different (integrated) renderers.
MIT License
22 stars 6 forks source link

erusev/parsedown included in roave/security-advisories #9

Open steffendietz opened 6 years ago

steffendietz commented 6 years ago

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. 😄

maglnet commented 6 years ago

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:

Con:

What do you think?

steffendietz commented 6 years ago

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.

maglnet commented 6 years ago

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 commented 6 years ago

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. :-)