wp_remote_get now sends the site url as referer #13664

wigglemuff · 2019-10-05T02:33:35Z

Changes proposed in this Pull Request:

Fixes this issue: Contact Info Widget: API keys with website restrictions not working correctly #13448

Testing instructions (Before the fix):

Go to widgets page and add Contact Info & Map widget by Jetpack
Enter Google's API key (without HTTP referrer restrictions) as explained in Jetpack docs. Embed works and we can see a preview after saving the widget
Remove the API key, save the widget (basically reset the widget and start fresh)
Go to Google API page and enable HTTP referrer restrictions to your site URL. Now on the widgets page, enter Google's API key again and save the widget. It returns an error Google Maps Platform rejected your request. This IP, site or mobile application is not authorized to use this API key.

Testing instructions (After the fix):

Go to Google API page and enable HTTP referrer restrictions to your site URL
Go to widgets page and add Contact Info & Map widget by Jetpack
Enter Google's API key and save the widget
It works as expected (displays a preview, no error).

Proposed changelog entry for your changes:

Fixes Google Maps referrer restrictions issue for "Contact Info & Map" widget.

jetpackbot · 2019-10-05T02:35:36Z

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: November 5, 2019.
Scheduled code freeze: October 29, 2019

Generated by 🚫 dangerJS against 9524dce

jeherve

In fd3c20d I fixed all PHPCS warnings in that file, including spacing and a trailing comma in the changes you've introduced.

This was a minor change though. Other than that, this seems to work well. 👍 I only have one minor remark.

I'll let @kbrown9 jump in and review as well. 👍

modules/widgets/contact-info.php

jeherve · 2019-10-07T08:20:10Z

modules/widgets/contact-info.php

+				$wp_remote_get_args = array(
+					'headers' => array( 'Referer' => site_url() ),
+				);
+				$response           = wp_remote_get( esc_url_raw( $path ), $wp_remote_get_args );



The actual changes are here.

Thanks Jer! :)

mdawaffe · 2019-10-07T17:07:03Z

@kbrown9 (you asked about referrers elsewhere). I looked a bit more, and the URL-as-Referer logic has been there since the beginning of the Requests library we use (since WordPress 4.6 in 2016):

WordPress/Requests@78d4f3c#diff-52f160e1af3ff131e027808a6c511447R50

Apparently it was taken from the previous library, SimplePie, which has included this code since 2006 (see the get_file() method further down the page there) as a means to get around anti-hotlinking.

https://github.com/simplepie/simplepie/blob/1.0_b1/simplepie.inc#L1406-L1410

So all HTTP requests sent by WordPress set the destination URL as the Referrer because SimplePie needed a way to bypass hotlinking prevention for some image requests thirteen years ago :)

Some related issues:

kbrown9

This looks good to me! I tested the branch using an API key with an HTTP restriction, and it works as expected.

Thanks for fixing this @codestor4!

matticbot · 2019-10-07T21:14:52Z

Caution: This PR has changes that must be merged to WordPress.com
Hello codestor4! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D33679-code before merging this PR. Thank you!

modules/widgets/contact-info.php

Co-Authored-By: Jeremy Herve <jeremy@jeremy.hu>

wigglemuff · 2019-10-11T18:15:23Z

Thanks everyone for reviewing this PR and sharing your thoughts. And thanks Jer for suggesting that commit. Sorry I am late to the party, was too occupied this week.

I think this should be good to go :)
Changing status now..

jeherve

This looks good to me. Merging.

* 7.9: Changelog * Update version number * Update stable tag and tested up to * Changelog: add #13530 * changelog: add #13578 * Changelog: add #13598 * Changelog: add entry for numerous block preview changes * Changelog: add #13599 * changelog: add #13541 * Changelog: add #13542 * Changelog: add #13331 * Changelog: add #13558 * Changelog: add #13409 * Changelog: add #13582 * Changelog: add #13600 * Changelog: add #13601 * Changelog: add #13595 * Changelog: add #12695 * Changelog: add #13009 * Changelog: add #13649 * Changelog: add #13450 * Changelog: add #13507 * Changelog: add #13658 * Changelog: add #13687 * changelog: add #13683 * Changelog: add #9323 * Changelog: add #13681 * Fix typos in readme * Add link to WordPress Beta Tester plugin * Changelog: add #13630 * Changelog: add #13695 * Changelog: add #13659 * Changelog: add #13716 * Changelog: add #13664 * Changelog: add #13682 * Changelog: add #13362 * Changelog: add #13563 * Add testing list for #13563 * Changelog: add #13735 * Changelog: add #13752 * Changelog: add #13624 * Changelog: add #13756 * Changelog: add #13745 * Changelog: add #13728 * Changelog: add #13779 * Changelog: add #13699 * Changelog: add #13804 * Changelog: add #13761 * Changelog: add #13637 * Changelog: add #13517 * Changelog: add #13521 * Changelog: add #13729 * Testing list: add testing instructions for #13729 * Changelog: add sync changes * Changelog: add #13807 * Changelog: add #13654 * Changelog: add #13795 * Changelog: add #13801 * Changelog: add #13818 * Changelog: add #13725 * Changelog: add #13831 * Changelog: add #13516 * Testing list: add Twenty Twenty instructions * Changelog: add #13799 * Changelog: add #13805 * Changelog: add #13688 * Changelog: add #13830

wp_remote_get now sends the site url as referer

673145e

wigglemuff added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Extra Sidebar Widgets [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 5, 2019

wigglemuff requested review from kbrown9 and a team October 5, 2019 02:33

wigglemuff self-assigned this Oct 5, 2019

matticbot added Touches WP.com Files labels Oct 5, 2019

fixed indentation

1b8460b

jeherve added this to the 7.9 milestone Oct 7, 2019

Fix all PHPCS warnings and add to whitelist

fd3c20d

jeherve requested changes Oct 7, 2019

View reviewed changes

modules/widgets/contact-info.php Outdated Show resolved Hide resolved

jeherve reviewed Oct 7, 2019

View reviewed changes

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 7, 2019

kbrown9 previously approved these changes Oct 7, 2019

View reviewed changes

jeherve requested changes Oct 11, 2019

View reviewed changes

modules/widgets/contact-info.php Outdated Show resolved Hide resolved

changed site_url function to home_url

9524dce

Co-Authored-By: Jeremy Herve <jeremy@jeremy.hu>

wigglemuff dismissed kbrown9’s stale review via 9524dce October 11, 2019 18:08

wigglemuff added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Oct 11, 2019

wigglemuff requested a review from jeherve October 14, 2019 08:33

jeherve approved these changes Oct 14, 2019

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 14, 2019

jeherve merged commit 6aefc26 into master Oct 14, 2019

jeherve deleted the fix/maps-referer branch October 14, 2019 08:44

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 14, 2019

jeherve mentioned this pull request Oct 14, 2019

Contact Info Widget: API keys with website restrictions not working correctly #13448

Closed

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Oct 23, 2019

jeherve added a commit that referenced this pull request Oct 23, 2019

Changelog: add #13664

abdaedb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wp_remote_get now sends the site url as referer #13664

wp_remote_get now sends the site url as referer #13664

wigglemuff commented Oct 5, 2019

jetpackbot commented Oct 5, 2019 •

edited

Loading

jeherve left a comment

jeherve Oct 7, 2019 •

edited

Loading

wigglemuff Oct 11, 2019

mdawaffe commented Oct 7, 2019

kbrown9 left a comment •

edited

Loading

matticbot commented Oct 7, 2019

wigglemuff commented Oct 11, 2019 •

edited

Loading

jeherve left a comment

wp_remote_get now sends the site url as referer #13664

wp_remote_get now sends the site url as referer #13664

Conversation

wigglemuff commented Oct 5, 2019

Changes proposed in this Pull Request:

Testing instructions (Before the fix):

Testing instructions (After the fix):

Proposed changelog entry for your changes:

jetpackbot commented Oct 5, 2019 • edited Loading

jeherve left a comment

Choose a reason for hiding this comment

jeherve Oct 7, 2019 • edited Loading

Choose a reason for hiding this comment

wigglemuff Oct 11, 2019

Choose a reason for hiding this comment

mdawaffe commented Oct 7, 2019

kbrown9 left a comment • edited Loading

Choose a reason for hiding this comment

matticbot commented Oct 7, 2019

wigglemuff commented Oct 11, 2019 • edited Loading

jeherve left a comment

Choose a reason for hiding this comment

jetpackbot commented Oct 5, 2019 •

edited

Loading

jeherve Oct 7, 2019 •

edited

Loading

kbrown9 left a comment •

edited

Loading

wigglemuff commented Oct 11, 2019 •

edited

Loading