Skip to content

LowExpiryCacheTime: improve handling of potential parse errors in eval-ed code#654

Merged
rebeccahum merged 1 commit intodevelopfrom
fix/lowexpirycachetime-parse-error-in-eval
Apr 15, 2021
Merged

LowExpiryCacheTime: improve handling of potential parse errors in eval-ed code#654
rebeccahum merged 1 commit intodevelopfrom
fix/lowexpirycachetime-parse-error-in-eval

Conversation

@jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Apr 15, 2021

Issue reported by @GaryJones in #628 (comment)

In rare cases, it is possible that the call to eval() with "safe" tokens only, would still result in a parse error.

An example of such a case is when the PHP 5.6 ** (T_POW) operator is used. PHPCS backfills the tokenization, which means that the operator is correctly recognized by the sniff as a "safe" token to use in the eval() statement.

However, when the sniff is being run on PHP 5.4/5.5, the eval() will also be run on PHP 5.4/5.5 and while PHPCS backfills the token, PHP does not, resulting in a parse error in the eval-ed code, upon which the eval() will return false.

This commit:

  • Silences the parse error notice.
  • Checks the output of the eval() for it being boolean false and if so, flags the statement for manual inspection, same as when any "non-safe" token is encountered.

…led code

Issue reported by GaryJones in Automattic/VIP-Coding-Standards 628#issuecomment-790653774

In rare cases, it is possible that the call to `eval()` with "safe" tokens only, would still result in a parse error.

An example of such a case is when the PHP 5.6 `**` (`T_POW`) operator is used. PHPCS backfills the tokenization, which means that the operator is correctly recognized by the sniff as a "safe" token to use in the `eval()` statement.

However, when the sniff is being run on PHP 5.4/5.5, the `eval()` will also be run on PHP 5.4/5.5 and while PHPCS backfills the token, PHP does not, resulting in a parse error in the `eval`-ed code, upon which the `eval()` will return `false`.

This commit:
* Silences the parse error notice.
* Checks the output of the `eval()` for it being boolean `false` and if so, flags the statement for manual inspection, same as when any "non-safe" token is encountered.
@jrfnl jrfnl added this to the 2.3.0 milestone Apr 15, 2021
@jrfnl jrfnl requested a review from a team as a code owner April 15, 2021 02:32
@rebeccahum rebeccahum merged commit c94e555 into develop Apr 15, 2021
@rebeccahum rebeccahum deleted the fix/lowexpirycachetime-parse-error-in-eval branch April 15, 2021 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants