Hacker News Reader

Comment by dugmartin | Hacker News Reader

I think the patch should have been to rip out eval() and build/use a shunting-yard parser/evaluator. A quick search found this: https://github.com/ircmaxell/php-math-parser

Leaving a call to eval() with user supplied input, no matter how well filtered, seems like way too big of a risk.

View on HN

Agreed, when you are on patch #4 because the previous patches weren't effective, and you can't verify patch #4 because of time constraints, it might be time to step back and reconsider the approach.

Sounds like they should just not be eval()ing user input regardless of how much sanitization they throw at it.

IMHO precedence climbing (a refactored version of recursive descent) would be much simpler and makes parsing seem almost trivial, since it can be made table-driven:

https://www.engr.mun.ca/~theo/Misc/exp_parsing.htm

Speaking of parsing simplicity and software design in general, you've coincidentally found a great example of how not to (ab)use OOP... one line of actual work wrapped in a dozen lines of boilerplate and replicated across multiple files with tiny variations:

https://github.com/ircmaxell/php-math-parser/tree/master/lib...

To put things into perspective, a precedence-climber for 13 levels of precedence from C's grammar, including a tokeniser, fits comfortably in <300 LoC of C, the bulk of that being the tokeniser. PHP's higher level nature (especially around string handling) would probably make a corresponding implementation even smaller.

Or use whatever tools are included in their language for safely evaluating strings (like ast.literal_eval in Python).

It's PHP there is none. In fact most languages don't include a safe way to evaluate strings out of the box.

> It's PHP there is none.

This seems to disprove that, no?

https://secure.php.net/manual/en/runkit.sandbox.php

Allow me to rephrase, no cheap way to evaluate arbitrary strings without spinning up another vm.

That’s not true either.

explain?

That's an extension. It doesn't come with PHP (not "out of the box").

Rails doesn’t come with a default install of Ruby. Xcode doesn’t come pre-installed on macOS. You have to install Chrome on every major desktop OS as it doesn’t come by default.

Installing and configuring your environment to your needs should be entry level stuff for a developer, so what was the point of your reply?

Reply on news.ycombinator.com