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

Make Wufoo safe for https usage and async #10017

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

marekhrabe
Copy link
Contributor

@marekhrabe marekhrabe commented Aug 13, 2018

Summary:
This removes the ssl argument of shortcode and replaces everything to work over https. SSL used to be a paid feature over at Wufoo but that changed 5 years ago https://www.wufoo.com/blog/making-wufoo-more-secure-https/

It also turns the old embed codes into their new async version and makes sure only a single form.js would be loaded even for multiple forms.

Testing instructions: Test with various wufoo embed shortcodes, all should work over https, regardless whether they set https argument (to true/false) or not at all.

Reviewers: paulbunkham

Reviewed By: paulbunkham

Subscribers: mdawaffe, paulbunkham, jblz

Tags: #touches_jetpack_files

Differential Revision: D16996-code

This commit syncs r179447-wpcom.

Proposed changelog entry for your changes:

Updated Wufoo Shortcode to always load over https and use async form embed.

Summary:
This removes the ssl argument of shortcode and replaces everything to work over https. SSL used to be a paid feature over at Wufoo but that changed 5 years ago https://www.wufoo.com/blog/making-wufoo-more-secure-https/

It also turns the old embed codes into their new async version and makes sure only a single `form.js` would be loaded even for multiple forms.

Test Plan: Test with various wufoo embed shortcodes, all should work over https, regardless whether they set https argument (to true/false) or not at all.

Reviewers: paulbunkham

Reviewed By: paulbunkham

Subscribers: mdawaffe, paulbunkham, jblz

Tags: #touches_jetpack_files

Differential Revision: D16996-code

This commit syncs r179447-wpcom.
@marekhrabe marekhrabe added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Dotcom Merge [Status] Tested on WP.com Touches WP.com Files labels Aug 13, 2018
@marekhrabe marekhrabe requested a review from a team as a code owner August 13, 2018 13:51
@marekhrabe marekhrabe requested a review from mdawaffe August 13, 2018 13:53
@jetpackbot
Copy link

jetpackbot commented Aug 13, 2018

This is automated (and not very smart btw) 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

Copy link
Member

@mdawaffe mdawaffe left a comment

Choose a reason for hiding this comment

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

Code looks reasonable. Do you have any specific tests I can try? I don't have a wufoo account. What URLs were you testing?

@abidhahmed abidhahmed added this to the 6.5 milestone Aug 14, 2018
@marekhrabe
Copy link
Contributor Author

[wufoo username="marekhrabe" formhash="zoz5s8o0qrgkec" autoresize="true" height="260" header="show" ssl="true"]

Wufoo now appends ssl="true" but that's not the case for many embeds generated in the past and they are missing that param forcing the old behavior to load over HTTP.

Some more forms (that I don't own so never submit!) 1ef08-pb

Copy link
Member

@mdawaffe mdawaffe left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good.

@mdawaffe mdawaffe merged commit 6bb6a9c into master Aug 14, 2018
@mdawaffe mdawaffe deleted the sync/marekhrabe/r179447-wpcom-1534168072 branch August 14, 2018 18:16
@mdawaffe mdawaffe removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Aug 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants