Integrate PHPCompatibility sniffs into CI toolchain

[danielbachhuber]

The PHPCompatibility sniffs help to identify PHP compatibility issues between versions. It would be helpful to integrate into the CI toolchain so Jetpack developers know when code changes have/introduce incompatible code.

In a data analysis project I'm doing right now, Jetpack is the most common failed plugin. The failure is caused by use of removed $HTTP_RAW_POST_DATA (explanation):

• jetpack/modules/markdown/easy-markdown.php

  Line 660 in edddd6b

  $message = new IXR_Message( $HTTP_RAW_POST_DATA );

• jetpack/class.jetpack.php

  Line 5478 in 2d531ca

  $this->HTTP_RAW_POST_DATA = $GLOBALS['HTTP_RAW_POST_DATA'];

• jetpack/class.jetpack-signature.php

  Line 65 in 32e2d44

  $body = isset( $GLOBALS['HTTP_RAW_POST_DATA'] ) ? $GLOBALS['HTTP_RAW_POST_DATA'] : null;

For further reference, here's a cache of Jetpack v5.4 scan results.

Related #4346

[zinigor]

This is great, thanks for letting us know about this! But I have a question: we have been looking at $HTTP_RAW_POST_DATA before when we were figuring out the changes needed to make Jetpack compatible with PHP 7.1. And it turns out that in these cases that global variable warning is a red herring - we repopulate $HTTP_RAW_POST_DATA to make sure we're compatible with PHP versions that still use it.

Do you know if there's a way to mark such occurences as false positives?

[danielbachhuber]

And it turns out that in these cases that global variable warning is a red herring - we repopulate $HTTP_RAW_POST_DATA to make sure we're compatible with PHP versions that still use it.

I see this only happening in Jetpack->wp_rest_authenticate():

jetpack/class.jetpack.php

Line 5350 in 2d531ca

$GLOBALS['HTTP_RAW_POST_DATA'] = file_get_contents( 'php://input' );

To my understanding, the two other contexts I identified (jetpack/modules/markdown/easy-markdown.php) and (jetpack/class.jetpack-signature.php) don't necessarily follow this codepath. Is this not the case?

Also, is there a reason you don't simply switch to file_get_contents( 'php://input' );?

Do you know if there's a way to mark such occurences as false positives?

Yes, you have two options:

• // @codingStandardsIgnoreLine to ignore a single following line.
• // @codingStandardsIgnoreStart and // @codingStandardsIgnoreEnd to ignore multiple lines.

[stale]

This issue has been marked as stale. This happened because:

• It has been inactive in the past 6 months.
• It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

[brbrr]

Fixed in #9905