Connection: Loose Comparison for Port Number in Signatures

[mdawaffe]

Changes proposed in this Pull Request:

When WordPress is hosted behind a reverse proxy, we ask site owners to add a X-Forwarded-Port header from the reverse proxy to the origin so that Jetpack can know what port to use in the signature's input.

We also allow site owners to define JETPACK_SIGNATURE__HTTPS_PORT and JETPACK_SIGNATURE__HTTP_PORT constants if adding a header is not possible.

Often, site owners will add the following snippet to their wp-config.php to make use of those constants:

define( 'JETPACK_SIGNATURE__HTTP_PORT', $_SERVER['SERVER_PORT'] );
define( 'JETPACK_SIGNATURE__HTTPS_PORT', $_SERVER['SERVER_PORT'] );

Unfortunately, we broke that snippet in #13489, since we moved to strict comparisons in:

• jetpack/packages/connection/legacy/class-jetpack-signature.php

  Line 95 in 97cc7bb

  $port = in_array( $host_port, array( 443, 80, $https_port ), true ) ? '' : $host_port;

• jetpack/packages/connection/legacy/class-jetpack-signature.php

  Line 103 in 97cc7bb

  $port = in_array( $host_port, array( 80, $http_port ), true ) ? '' : $host_port;

$_SERVER['SERVER_PORT'] is a string in most environments, and the new code demands integers.

Switch back to loose comparison.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

No: bug fix

Testing instructions:

There's not a good way to test this unless you have a site running on a custom port behind a reverse proxy running on a normal port :(

Proposed changelog entry for your changes:

• Fix communication between Jetpack sites and WordPress.com for some sites hosted on non-standard ports.

[mdawaffe]

If we do a point release, this should be in it.

[jetpackbot]

	Warnings
⚠️	pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against b37f494

[mdawaffe]

Sorry about the [not verified]. I did actually verify, then had to do some rebase shenanigans :)

[brbrr]

Jetpack e2e tests were complaining about that change, so we figured this fix: #13523, which seems not fixing all the issues

[jeherve]

LGTM. Merging.

[jeherve]

Cherry-picked to branch-7.9 in c15baec