yeslogic / prince-php-wrapper

PHP wrapper class for Prince HTML to PDF formatter
https://www.princexml.com
MIT License
16 stars 7 forks source link

Use `escapeshellarg` instead of custom methods? #1

Closed IllyaMoskvin closed 2 years ago

IllyaMoskvin commented 4 years ago

Thank you for sharing this wrapper! I'm skimming through the code, and I noticed that it has some custom methods to escape commandline arguments:

https://github.com/yeslogic/prince-php-wrapper/blob/45fb4575bacb826d9db11e17f9b0a9033a68110f/prince.php#L1136-L1139

https://github.com/yeslogic/prince-php-wrapper/blob/45fb4575bacb826d9db11e17f9b0a9033a68110f/prince.php#L1141-L1143

https://github.com/yeslogic/prince-php-wrapper/blob/45fb4575bacb826d9db11e17f9b0a9033a68110f/prince.php#L1187-L1188

I was wondering: would escapeshellarg work better here? As a built-in function, I'm assuming it's the more secure option. Or is there something about these arguments specifically that requires the use of custom methods?

mikeday commented 4 years ago

We switched away from escapeshellarg some years ago due to problems with special characters in command-line arguments on Windows specifically, but we should probably retest that to see if it's still an issue or not.

adrianwong commented 2 years ago

Apologies for the delay in addressing this, @IllyaMoskvin.

I did some reading into the escapeshellarg issues @mikeday has mentioned, and (per my understanding) a number of issues remain unaddressed.

I've resorted to using a third-party library https://github.com/johnstevenson/winbox-args, which tries to provide a more robust alternative to escapeshellarg. This code is now in use in the 1.2.0 release of this library.