Skip to content

Connection: Fix PHPCS errors in Jetpack_Signature#13489

Merged
kraftbj merged 1 commit into
masterfrom
fix/phpcs-connection-signature
Sep 17, 2019
Merged

Connection: Fix PHPCS errors in Jetpack_Signature#13489
kraftbj merged 1 commit into
masterfrom
fix/phpcs-connection-signature

Conversation

@tyxla
Copy link
Copy Markdown
Member

@tyxla tyxla commented Sep 16, 2019

This PR fixes all the PHPCS errors and warnings in the Jetpack_Signature class.

Changes proposed in this Pull Request:

  • Connection: Fix PHPCS errors in Jetpack_Signature

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

  • Part of the connection package work.

Testing instructions:

  • Not necessary, this is mostly adding docs.
  • If you really wish, you can do some smoke testing.

Proposed changelog entry for your changes:

    • Connection: Fix PHPCS errors in Jetpack_Signature

@tyxla tyxla added [Status] Needs Review This PR is ready for review. [Type] Janitorial [Pri] Low Connect Flow Connection banners, buttons, ... labels Sep 16, 2019
@tyxla tyxla added this to the 7.8 milestone Sep 16, 2019
@tyxla tyxla requested review from a team September 16, 2019 22:36
@tyxla tyxla self-assigned this Sep 16, 2019
@github-actions github-actions Bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Sep 16, 2019
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Sep 16, 2019

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: October 1, 2019.
Scheduled code freeze: September 24, 2019

Generated by 🚫 dangerJS against b5f1f8b

jeherve
jeherve approved these changes Sep 17, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Thanks for that!

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Sep 17, 2019
@kraftbj kraftbj merged commit cb98c0f into master Sep 17, 2019
@kraftbj kraftbj deleted the fix/phpcs-connection-signature branch September 17, 2019 15:54
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 17, 2019
@jeherve jeherve removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Sep 20, 2019
@tyxla tyxla mentioned this pull request Sep 24, 2019
Merged
mdawaffe added a commit that referenced this pull request Nov 22, 2019
@mdawaffe
[not verified] Connection: Loose Comparison for Port Number in Signat…
b37f494 
…ures

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:
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L95
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L102

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

Switch back to loose comparison.
@mdawaffe mdawaffe mentioned this pull request Nov 22, 2019
Merged
jeherve pushed a commit that referenced this pull request Nov 25, 2019
@mdawaffe @jeherve
Connection: Loose Comparison for Port Number in Signatures (#14111)
1297eee 
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:
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L95
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L102

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

Switch back to loose comparison.
jeherve pushed a commit that referenced this pull request Nov 25, 2019
@mdawaffe @jeherve
[not verified] Connection: Loose Comparison for Port Number in Signat…
c15baec 
…ures (#14111)

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:
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L95
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L102

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

Switch back to loose comparison.
jeherve pushed a commit to Automattic/jetpack-connection that referenced this pull request Nov 25, 2019
@mdawaffe @jeherve
Connection: Loose Comparison for Port Number in Signatures (#14111)
4834877 
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
Automattic/jetpack#13489, since we moved to strict
comparisons in:
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L95
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L102

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

Switch back to loose comparison.
zinigor pushed a commit to Automattic/jetpack-sync that referenced this pull request Dec 30, 2019
@mdawaffe @jeherve
Connection: Loose Comparison for Port Number in Signatures (#14111)
92097a1 
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
Automattic/jetpack#13489, since we moved to strict
comparisons in:
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L95
* https://github.com/Automattic/jetpack/blob/97cc7bb9b26d4184ba4915efd5928e59d4456b38/packages/connection/legacy/class-jetpack-signature.php#L102

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

Switch back to loose comparison.
@leogermani leogermani mentioned this pull request Nov 4, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kraftbj kraftbj kraftbj approved these changes

@jeherve jeherve jeherve approved these changes

Assignees

@tyxla tyxla

Labels

Connect Flow Connection banners, buttons, ... [Pri] Low [Type] Janitorial

Projects

None yet

Milestone

7.8

Development

Successfully merging this pull request may close these issues.

5 participants

@tyxla @jetpackbot @kraftbj @jeherve @matticbot