WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 2 months ago

Last modified 2 months ago

#48077 closed defect (bug) (fixed)

Plugin install fails from popup modal on localhost installs

Reported by: garrett-eclipse Owned by: desrosj
Milestone: 5.3 Priority: low
Severity: normal Version: 4.6
Component: Upgrade/Install Keywords: has-screenshots has-patch commit
Focuses: javascript, administration Cc:
PR Number:

Description

Hello,

Stemming from #48071 came across the fact that the 'Install Now' button on the plugin details popup modal fails silently, but only on localhost installs. My install specifically was MAMP localhost:8888

It was found uploading the same site to a server resolves the issue so must be an issue with triggering _parent from a iframe.

Cheers
P.S. Opening just to investigate and see if even possible to address, I have a feeling it's just a limitation of local development.

Attachments (2)

237ae944b2bbdc30437ac12f8fd08a81 (1).gif (3.1 MB) - added by garrett-eclipse 3 months ago.
Install issue on localhost from plugin popup modal iframe
48077.diff (541 bytes) - added by pierlo 3 months ago.

Change History (15)

@garrett-eclipse
3 months ago

Install issue on localhost from plugin popup modal iframe

#1 @garrett-eclipse
3 months ago

  • Keywords has-screenshots added

@pierlo
3 months ago

#2 @pierlo
3 months ago

  • Keywords has-patch added

In 48077.diff, use host instead of hostname for retrieving the domain.

When verifying the origin for postMessage events, the port was not considered, so any instance of WordPress being served from a custom port (not 80 or 443) won't work.

#3 @garrett-eclipse
3 months ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 5.3

Thanks @pierlo that worked like a charm, and testing on a non-localhost there's also no issues there. Marking for commit and adding to 5.3

#4 @afragen
3 months ago

@garrett-eclipse is it possible the issue is with the port number getting parsed or sanitized somewhere? When I did all the plugin PHP compatibility patches I was using Local by Flywheel and was able to update from all these iframes, etc without an issue.

Last edited 3 months ago by afragen (previous) (diff)

#5 @garrett-eclipse
3 months ago

Hi @afragen

I do believe it has to do with the port being present on the URL as on Local by Flywheel it's no issue and at work where I have MAMP Pro w/ domain mapping it's also no issue it's just at home where MAMP basic is at localhost:8888 as localhost is occupied by apache server.

As @pierlo mentioned 'When verifying the origin for postMessage events, the port was not considered, so any instance of WordPress being served from a custom port (not 80 or 443) won't work.'

The patch and change to 'use host instead of hostname for retrieving the domain.' has addressed this without introducing any issue as yet on other configurations such as those using standard ports.

#6 @pierlo
3 months ago

What @garrett-eclipse said. Given the origin http://localhost:8888:

window.location.host produces localhost:8888
window.location.hostname produces localhost

#7 follow-up: @afragen
3 months ago

@garrett-eclipse I did some digging and you’ve already solved this issue. 😉

https://github.com/afragen/github-updater/issues/491

function allow_unsafe_urls ( $args ) {
	$args['reject_unsafe_urls'] = false;
	return $args;
}
add_filter( 'http_request_args', 'allow_unsafe_urls' );

#8 in reply to: ↑ 7 @garrett-eclipse
3 months ago

Replying to afragen:

@garrett-eclipse I did some digging and you’ve already solved this issue. 😉
https://github.com/afragen/github-updater/issues/491

function allow_unsafe_urls ( $args ) {
	$args['reject_unsafe_urls'] = false;
	return $args;
}
add_filter( 'http_request_args', 'allow_unsafe_urls' );

Thanks @afragen I don't believe that's related to this and wouldn't think we'd want a filter to support local development.

That being said I did attempt it but was unsuccessful with TwentyNineteen and applying that filter (2016 wow how time flies).

The issue as illustrated by the fix in 48077.diff is all JS related so isn't being run by the reject_unsafe_urls filters. The issue in the JS is the check on the postMessage event which didn't take into account the potential of there being a port on it. The patch addresses this discrepancy in the JS conditional check by allowing the port to be present in expectedOrigin by using document.location.host instead of document.location.hostname.

#9 @afragen
3 months ago

It seems like @pierlo‘s patch is a good solution.

Yes, time flies. 😉

#10 @desrosj
2 months ago

  • Owner set to desrosj
  • Status changed from new to reviewing

#11 @desrosj
2 months ago

  • Version set to 4.6

Thanks, everyone! This looks to have been introduced in [37714].

#12 @desrosj
2 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 46348:

Upgrade/Install: Fix the Install Now button in the plugin overlay when running WordPress on a specific port.

Props garrett-eclipse, pierlo, afragen.
Fixes #48077.

#13 @garrett-eclipse
2 months ago

Thanks @desrosj I can confirm I'm now able to install plugins using the modal on Trunk. I appreciate the fix @pierlo

Note: See TracTickets for help on using tickets.