Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connection: Make sure port is an integer #13523

Merged
merged 1 commit into from
Sep 24, 2019
Merged

Conversation

tyxla
Copy link
Member

@tyxla tyxla commented Sep 24, 2019

Sometimes HTTP_X_FORWARDED_PORT can be a string, which after #13489 will cause issues with signature generation. This PR converts those ports to integer, to ensure proper comparison.

Changes proposed in this Pull Request:

  • Connection: Make sure port is an integer

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

  • Just a bugfix

Testing instructions:

  • cc @brbrr for testing - this should be fixing the connection e2e tests.

Proposed changelog entry for your changes:

  • Connection: Make sure port is an integer

@tyxla tyxla added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. [Pri] Normal Connect Flow Connection banners, buttons, ... labels Sep 24, 2019
@tyxla tyxla added this to the 7.8 milestone Sep 24, 2019
@tyxla tyxla requested review from brbrr and a team September 24, 2019 07:46
@tyxla tyxla self-assigned this Sep 24, 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 24, 2019
Copy link
Contributor

@brbrr brbrr left a comment

Choose a reason for hiding this comment

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

Looks good. let's wait for e2e tests in CI

@jetpackbot
Copy link
Collaborator

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 1433fea

@brbrr
Copy link
Contributor

brbrr commented Sep 24, 2019

Yay, tests are green! merging

@brbrr brbrr merged commit 829c19c into master Sep 24, 2019
@brbrr brbrr deleted the fix/signature-port-conversion branch September 24, 2019 07:55
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review This PR is ready for review. labels Sep 24, 2019
@brbrr brbrr removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Sep 24, 2019
@brbrr
Copy link
Contributor

brbrr commented Sep 24, 2019

Released in automattic/[email protected]

@jeherve
Copy link
Member

jeherve commented Sep 24, 2019

Cherry-picked to branch-7.8 in c96f340

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Connect Flow Connection banners, buttons, ... [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants