#47577 closed enhancement (fixed)
Detect HTTPS support and provide guidance
Reported by: | flixos90 | Owned by: | flixos90 |
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Security | Keywords: | has-patch has-unit-tests has-dev-note |
Focuses: | administration | Cc: |
Description (last modified by )
Of all the WordPress sites today, 63.4% are using HTTPS. While this is already better than the average for the entire web, it is far from optimal. More and more modern web APIs require usage of HTTPS, let alone the security implications of not using it.
In order to close that gap, WordPress should do better to actively recommend administrators to switch their non-HTTPS site to use HTTPS, especially if their current environment already technically supports it.
In order to provide accurate recommendations to site owners about switching their site to HTTPS, we need to know whether HTTPS is even supported by their server and domain. This has been reliably detected in the PWA plugin for a while, and similar logic could be used in core.
Based on the result of the HTTPS support detection, we would recommend one of the following:
- If supported, recommend to change the WordPress site URL, as that's all that's needed.
- If not supported, recommend talking to the web host about enabling HTTPS.
This provide more accurate recommendations for the respective situation a site is in. Then, in separate follow-up tickets, we should look at simplifying the migration from HTTP to HTTPS itself which today is far too complex for the majority of WordPress users.
Attachments (5)
Change History (62)
#2
@
5 years ago
It would be nice to allow filtering the Site Health action link/button for talking to your host about HTTPS in the same way we do for the PHP upgrade notices.
Hosts will have differing documentation about how to accomplish it on their environments and they may want to link directly to that.
#4
in reply to:
↑ 1
@
5 years ago
Replying to flixos90:
- If HTTPS is active, filter and replace non-HTTP links to the site in content with their HTTPS counterparts.
I suggest also adding a upgrade-insecure-requests
CSP directive to automatically handle this outside fo the content: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/upgrade-insecure-requests
#5
follow-up:
↓ 6
@
5 years ago
@earnjam
It would be nice to allow filtering the Site Health action link/button for talking to your host about HTTPS in the same way we do for the PHP upgrade notices.
I like that suggestion, however I'd prefer if we approached this iteratively and opened a follow-up issue to make that URL filterable. The complexity we've learned about during the Servehappy project is that we probably wouldn't want them to replace the wordpress.org support URL, so we'd need to add more content and tweak the UI to allow for that, which would require more discussion.
@westonruter
I suggest also adding a
upgrade-insecure-requests
CSP directive to automatically handle this outside fo the content
That's worth exploring. I'm wondering whether that would cause problems with URLs pointing to external websites, that may still not be on HTTPS though - how does the directive deal with images or links from such websites? The other concern is that in order to add CSP headers into core, it may be better to work on a simple centralized solution as a developer API that would allow managing those directives.
#6
in reply to:
↑ 5
;
follow-up:
↓ 7
@
5 years ago
Replying to flixos90:
I suggest also adding a
upgrade-insecure-requests
CSP directive to automatically handle this outside fo the content
That's worth exploring. I'm wondering whether that would cause problems with URLs pointing to external websites, that may still not be on HTTPS though - how does the directive deal with images or links from such websites? The other concern is that in order to add CSP headers into core, it may be better to work on a simple centralized solution as a developer API that would allow managing those directives.
Hyperlinks to other websites would be ignored since merely linking to another site does not cause a request. Images, videos, iframes, and other resources would need to be upgraded even on external domains as otherwise there would be insecure mixed content warnings.
Per MDN:
The upgrade-insecure-requests directive will not ensure that users visiting your site via links on third-party sites will be upgraded to HTTPS for the top-level navigation
#7
in reply to:
↑ 6
;
follow-up:
↓ 8
@
5 years ago
Replying to westonruter:
Hyperlinks to other websites would be ignored since merely linking to another site does not cause a request. Images, videos, iframes, and other resources would need to be upgraded even on external domains as otherwise there would be insecure mixed content warnings.
How would you handle the case where external images are not available via HTTPS? Or are you thinking that perhaps upgrade-insecure-requests
could be optional? Or maybe require handling first and/or logging the issues that need fixing?
Per MDN:
Note that, if the requested resource is not actually available via HTTPS, the request will fail without any fallback to HTTP
#8
in reply to:
↑ 7
;
follow-up:
↓ 9
@
5 years ago
Replying to miinasikk:
How would you handle the case where external images are not available via HTTPS? Or are you thinking that perhaps
upgrade-insecure-requests
could be optional? Or maybe require handling first and/or logging the issues that need fixing?
Yeah, I'm not sure. I think ultimately the insecure external assets would just need to fail. It's something the site owner (or theme/plugin developer) would need to fix (or find a replacement).
Without upgrade-insecure-requests
browsers will still load insecure images. Other insecure subresources, scripts in particular, are blocked entirely. So it seems better to have this in place so that the upgrade is performed for the majority of subresources that should be available over HTTPS.
It would be nice if upgrade-insecure-requests
could be configured to only upgrade non-image subresources, leaving images to flag insecure content warnings, but that doesn't seem to be the case.
So while upgrade-insecure-requests
should be the default, it makes sense that a developer should be able to turn off that default behavior and instead opt to find insecure requests via:
Content-Security-Policy-Report-Only: default-src https:; report-uri /endpoint
This would require the support of a plugin like Reporting API. It's also not something that should be the default for all sites.
The 80% solution here—for the majority of users who don't know how to debug things like this—seems to be just to upgrade-insecure-requests
. This should be come less and less of a problem as more sites support HTTPS.
#9
in reply to:
↑ 8
@
5 years ago
Replying to westonruter:
The 80% solution here—for the majority of users who don't know how to debug things like this—seems to be just to
upgrade-insecure-requests
. This should be come less and less of a problem as more sites support HTTPS.
If we would add the upgrade-insecure-requests
e.g. by adding 'wp_headers'
filter to https-detection.php
then this would mean that all the sites that already are using HTTPS would benefit from that. I'm wondering about the cases though where there might be hundreds of posts of which some might use third-party resources which might not support HTTPS, and this would happen without any notice about it after WP update. Not sure if there is a good way to inform about this behavior change, other than stating it in the added Security section. Or do you think it should probably be acceptable as-is since it would work for 80% of the cases? Not sure what's the general policy for these cases, is 80% rule the policy? :)
So while
upgrade-insecure-requests
should be the default, it makes sense that a developer should be able to turn off that default behavior and instead opt to find insecure requests via:
Content-Security-Policy-Report-Only: default-src https:; report-uri /endpointThis would require the support of a plugin like Reporting API. It's also not something that should be the default for all sites.
We could add a filter which by default would add the upgrade-insecure-requests
but when set to the opposite value (false
vs true
depending on the filter) would add Content-Security-Policy-Report-Only
instead.
#10
@
5 years ago
If a site switches to HTTPS but they have HTTP resources, then (as I understand):
- Without
upgrade-insecure-requests
, insecure scripts will be blocked and insecure media will cause warnings. - With
upgrade-insecure-requests
, all insecure resources will automatically be updated to HTTPS. If the hosts for the resources support HTTPS, then everything will work seamlessly. If a hosted resource is not available on HTTPS (scripts and media), then it will fail.
In both cases, the user would need to check their site to check for problems (unless it's for a new install). But my expectation is that upgrade-insecure-requests
will generally result in fewer errors, though the ones that do occur (e.g. for images) will be harder (failures instead of warnings). So 2 seems like a better 80% solution.
@
5 years ago
Includes adding Content-Security-Policy header with filter to opt out for Content-Security-Policy-Report-Only
#11
@
5 years ago
Added the header + also information about Content-Security-Policy
. That will only be visible if the HTTPS is not configured correctly though, the sites already using HTTPS won't see this. Not sure if this makes sense. Thoughts?
#12
follow-up:
↓ 13
@
5 years ago
@westonruter
If a hosted resource is not available on HTTPS (scripts and media), then it will fail.
I'm okay with including upgrade-insecure-requests
, but your point here leads me to believe that we should do a bit more to make sure it doesn't break users' sites. How about we also issue a request to a random media file, and potentially even a core asset, to make the same check (only if the root URL is different, e.g. when using a CDN)? Only if all of them support HTTPS, we'll tell the user that their site supports HTTPS. It is admittedly a special case, but probably enough people are using a CDN (and hopefully they're all good enough to support HTTPS) to make this worth having.
@miinasikk
Added the header + also information about
Content-Security-Policy
.
I'd prefer to not expose this to the user, at least not in here - it's very technical information and most folks will have no idea what it is. If we want to expose it, then I'd prefer just doing it in the Info part of Site Health, with all the other technical data.
After reading the previous discussion and reviewing the patch, I'd prefer if we just set the upgrade-insecure-requests
value in the filter and not bothered about the very specific report-only
cases. You'd need to use a filter to modify the behavior anyway, and you could just as well remove our filter if you wanted to do so. Let's keep the code simple here, covering the vast majority of cases. What we should probably do though is check, if the Content-Security-Policy
header is already set, and if so append our value (maybe we should even check whether it's not already set). We don't wanna override other CSP headers people may be using already.
#13
in reply to:
↑ 12
@
5 years ago
Replying to flixos90:
How about we also issue a request to a random media file, and potentially even a core asset, to make the same check (only if the root URL is different, e.g. when using a CDN)? Only if all of them support HTTPS, we'll tell the user that their site supports HTTPS. It is admittedly a special case, but probably enough people are using a CDN (and hopefully they're all good enough to support HTTPS) to make this worth having.
Are you suggesting that we should check if a random media file that's already on the site supports HTTPS and if it doesn't then consider the site not supporting HTTPS?
Some concerns with that:
- We would check just one random media file. This would mean that it could happen that the chosen file is from CDN but it could also be from some other random source / be a leftover or irrelevant and not accurately reflect the site being ready for HTTPS.
- How would you choose the random media file, could it differ at different times and have different results?
- This wouldn't really reflect accurately if the site supports HTTPS, having external resources not supporting HTTPS means that the external site isn't supporting HTTPS, not the local.
If the CDN doesn't support HTTPS then maybe it would be good to change the service, too.
Also, the sites that aren't already supporting HTTPS aren't going to have the CSP header anyway -- these sites would actually need to take steps to convert the site to HTTPS where some issues can be expected. It would probably be unexpected for the sites already supporting HTTPS when the resources suddenly display broken, for those, however, the information about being or not being HTTPS ready wouldn't be relevant anymore. WDYT?
I'd prefer to not expose this to the user, at least not in here - it's very technical information and most folks will have no idea what it is. If we want to expose it, then I'd prefer just doing it in the Info part of Site Health, with all the other technical data.
Fair point that it's very technical information and out of the place. The technical information does make sense under Info more, however, we should first actually check if the header hasn't been overwritten by a filter before displaying the information about it. Also, if we already display the CSP header information then should others be displayed as well? Perhaps you're right and it's not that important to display it in such detail.
It would be good to have information under the Status with the HTTPS information though, however, not sure what would be a good way to tell that "External resources that don't support HTTPS might break after switching to HTTPS -- make sure that you don't have those!". Do you think it'd be unnecessary information for the user and we could just skip all the information part? Perhaps I'm overthinking.
After reading the previous discussion and reviewing the patch, I'd prefer if we just set the
upgrade-insecure-requests
value in the filter and not bothered about the very specificreport-only
cases. You'd need to use a filter to modify the behavior anyway, and you could just as well remove our filter if you wanted to do so.
That's true, however, I think that the report-only case would be helpful for transitioning, we could document the filter better to make it more clear what's it for, it's very likely that the report-only header is not something that everyone is aware of and the header would be just removed instead of trying to transition. So having that filter could help.
What we should probably do though is check, if the
Content-Security-Policy
header is already set, and if so append our value (maybe we should even check whether it's not already set). We don't wanna override other CSP headers people may be using already.
It wouldn't make sense to just append our value without knowing what's there before, that would cause duplicate directives and not really work (e.g. having default-src twice). Checking first if it already exists and adding the header only then is a good point, no need to overwrite the exising efforts.
#14
@
5 years ago
@miinasikk @westonruter
I wonder whether we should take a step back here and not upgrade-insecure-requests
for now, except those that go against the actual site URL. While this can just as well cause issues because of e.g. media or assets on a separate host, I think it keeps the work here more scoped. Exploring upgrade-insecure-requests
could be the second step - I'm afraid this gets lost in a can of worms otherwise.
Arguably, most WordPress sites serve all their files from the same origin, so for those the simple HTTP to HTTPS replacement should work. The complexity of checking for files that are served from different origins leads me to think that we should defer that work for now. Potentially it even is plugin territory: For example, a plugin hooking media up a CDN could (if that CDN doesn't already use HTTPS anyway) make use of wp_is_using_https()
to act accordingly.
Last but not least, we need to keep in mind that users can at least change their URLs back to HTTP, should any resource unexpectedly cause their site to break.
#15
@
5 years ago
As noted in No More Mixed Messages About HTTPS, Chromium is going to start blocking all non-HTTP subresources on pages loaded over HTTPS:
In a series of steps starting in Chrome 79, Chrome will gradually move to blocking all mixed content by default. To minimize breakage, we will autoupgrade mixed resources to https://, so sites will continue to work if their subresources are already available over https://. Users will be able to enable a setting to opt out of mixed content blocking on particular websites, and below we’ll describe the resources available to developers to help them find and fix mixed content.
It states that HTTP-resources will be autoupgraded to HTTPS, at least audio, video, and images. However, it doesn't mention scripts or iframes.
So if WordPress served upgrade-insecure-requests
it would seem that this would preempt what Chromium is already doing. It would prevent broken media as well eventually prevent broken scripts and iframes which would not get automatically upgraded, as far as I understand.
I'm not sure what Firefox or Safari are planning in this regard.
#16
@
4 years ago
Revisiting this ticket, I believe we're suffering of some scope creep here, so it would make sense to split this ticket into multiple (closely related, but different) efforts:
- Detect HTTPS support and provide guidance
- Streamline migrating from HTTP to HTTPS
- Upgrade insecure requests when site is using HTTPS
Technically the above order would also be a dependency chain for these three, but discussion on the different efforts can happen separately and focus on one topic.
I'm going to move forward and simplify this ticket to cover 1., then open new tickets for 2. and 3.
#17
@
4 years ago
- Component changed from Administration to Security
- Description modified (diff)
- Focuses administration added
- Keywords needs-patch added; 2nd-opinion has-patch removed
- Milestone changed from Awaiting Review to Future Release
- Summary changed from Streamline detecting and enabling HTTPS to Detect HTTPS support and provide guidance
This ticket was mentioned in PR #568 on WordPress/wordpress-develop by felixarntz.
4 years ago
#18
- Keywords has-patch added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/47577
## Screenshots
HTTPS already supported:
<img width="1064" alt="Screenshot 2020-10-02 at 14 01 56" src="https://user-images.githubusercontent.com/3531426/94969916-67638100-04b8-11eb-9ca7-892b4f23ac61.png">
HTTPS not supported:
<img width="1065" alt="Screenshot 2020-10-02 at 14 02 57" src="https://user-images.githubusercontent.com/3531426/94969972-7d714180-04b8-11eb-8a52-b176972d3c14.png">
#19
@
4 years ago
- Keywords has-unit-tests added; needs-unit-tests removed
In the latest commit in the PR https://github.com/WordPress/wordpress-develop/pull/568, I've added unit test coverage. I've also added a fix where now the siteurl
option is accessed directly in wp_is_https_supported()
. This is necessary because otherwise (via site_url()
) the URL's scheme will be adjusted based on whether the current request uses HTTPS or not, which shouldn't affect the URL set here. @westonruter What do you think about this? I would think this may be an issue for the PWA plugin similarly?
#20
@
4 years ago
@flixos90 Yeah, calling site_url()
to determine the underlying URL scheme seems problematic since set_url_scheme()
will override that underlying scheme.
So it seems that get_option('siteurl')
will work, and it will also account for the case where someone has defined the siteurl
via the WP_SITEURL
constant, since this is enforced via a lower-level option_siteurl
filter rather than the site_url
filter.
The only concern I have is if a site is using the site_url
filter to manipulate the siteurl
to be something else rather than what is returned by get_option('siteurl')
. I suppose that would be a rare scenario. But one possibility to ensure to account for that would be to apply the filters yourself:
<?php $site_url = apply_filters( 'site_url', get_option( 'siteurl' ), '/', null, get_current_blog_id() );
#21
@
4 years ago
- Milestone changed from Future Release to 5.7
- Owner set to flixos90
- Status changed from new to assigned
#22
follow-up:
↓ 23
@
4 years ago
@westonruter Good point, I've updated the PR to manually re-run the filter. It's a bit of a hack, but ensures consistent functionality. Altering the behavior of siteurl()
itself to use the "real" URL scheme by default would be a breaking change and at least controversial, so that's out of scope for this ticket.
I plan to commit this to trunk in a few days.
#23
in reply to:
↑ 22
@
4 years ago
Replying to flixos90:
@westonruter Good point, I've updated the PR to manually re-run the filter. It's a bit of a hack, but ensures consistent functionality. Altering the behavior of
siteurl()
itself to use the "real" URL scheme by default would be a breaking change and at least controversial, so that's out of scope for this ticket.
Looks good.
I plan to commit this to trunk in a few days.
I've left a review.
This ticket was mentioned in Slack in #core by flixos90. View the logs.
4 years ago
#25
@
4 years ago
As @westonruter already did a great run-through of recommended improvements code-wise, I'll just comment on the Site Health implementation it self.
I recall it was mentioned that having a way to reach the host would be convenient here, I believe this was brought up in the core chat while the ticket was discussed, and I think that might make sense. There's already a constant for PHP updates that hosts can make use of to direct users to their own documentation, and I think if HTTPS isn't available not all users know where to go /you'd be surprised how many don't actually know who their webhost is).
Would it make sense to introduce a new method for hosts to provide the URL for their support system (hosts could then filter this how they like to either be generic, or tailored for each clients site, that's up to them, but we'll facilitate it). There are various scenarios where you'd want to involve a host for support, so this is a feature that could be re-used a fair bit, also by plugins or themes looking to make use of functionality that is unsupported, and they could then give their users an actionable path forward.
The existing scenario supports an environment variable of WP_DIRECT_UPDATE_PHP_URL
, having WP_HOST_SUPPORT_URL
or similar (name not set in stone, just for discussion) might then be useful, with a helper function?
Other than that, I think the Site Health implementation seems simple enough, I'm curious if this might be a great first step into introducing direct actions? This would mean having the action link, which currently takes you to the options page, perform the options update for you, it could then verify the page still loads as expected wit ha loopback (I do love those loopbacks), and revert and direct users to host support if it fails, as that would indicate something is wrong 🤔
#26
@
4 years ago
@Clorith
The existing scenario supports an environment variable of
WP_DIRECT_UPDATE_PHP_URL
, havingWP_HOST_SUPPORT_URL
or similar (name not set in stone, just for discussion) might then be useful, with a helper function?
Thanks for bringing this up, I like it though I think a HTTPS update-specific constant would be more suitable here, and closer to the use-case, similar to how we have one for the PHP update. A general support URL could also be handy in many cases, but I think that's less specific to this ticket and would be better as its own. I suggest we introduce a WP_DIRECT_UPDATE_HTTPS_URL
constant for e.g. hosts to modify.
Other than that, I think the Site Health implementation seems simple enough, I'm curious if this might be a great first step into introducing direct actions? This would mean having the action link, which currently takes you to the options page, perform the options update for you, it could then verify the page still loads as expected wit ha loopback
I like that idea as well. However, WordPress itself IMO should first do a better job of facilitating this update, as only updating the URLs is not feasible. #51437 is the ticket to do that, which is gonna be the next focus after this here is committed.
I suggest we apply both of your suggestions for #51437 which is about improving the update experience, where this here is more about improving detection.
#29
@
4 years ago
- Keywords needs-dev-note added
Requiring a dev-note, in combination with #51437 which is next.
#31
follow-up:
↓ 35
@
4 years ago
A few thoughts.
Some of these new function names do not follow closely the recommended pattern. Relatedly, that means my admittedly aging brain has a harder-than-expected time understanding what they are intended to do without additional study.
- wp_update_https_detection_errors - update what errors, from where?
- wp_schedule_https_detection - schedule how and for what? Cron? Deletion/Addition?
- wp_is_owned_html_output - Owned by who? Me?
wp_update_https_detection_errors seems to be named what it is because it calls update_option() internally (which is an assumption on my part) and doesn't provide much benefit to include that detail in the function name, as the storage mechanism is not contextually relevant. It could be: wp_detect_https_errors, which is vague about its storage but assertive about what it does.
wp_schedule_https_detection is following the pattern laid out by other wp_schedule_ functions, which are inconsistently named as well. update_network_counts, update_checks, delete_old_privace_export_files. Youch. Maybe wp_schedule_update_https_detection is better, even though it's not great?
wp_is_owned_html_output is introducing a new global term related to content ownership. I'd like this to be reconsidered to avoid any possible future collisions with other APIs like roles & caps, multisite networks, etc... For example, #5942 wants to introduce an "Owner" role. If that happens, this function name would probably be undesirable. wp_is_local_html_output maybe? But "local" has similar connotations with development environments... Not sure about this one.
These functions are also all marked as @access private
but not prefixed with an underscore. Natch, this is broadly inconsistent, but has been enforced as recently as 5.5.0 with _wp_register_meta_args_allowed_list()
. Just want to make doubly sure this was considered and makes sense for these here.
hellofromtonya commented on PR #568:
4 years ago
#32
Merged with changeset https://core.trac.wordpress.org/changeset/49904
#33
@
4 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening as per above
This ticket was mentioned in Slack in #hosting-community by javier. View the logs.
4 years ago
#35
in reply to:
↑ 31
@
4 years ago
Replying to johnjamesjacoby:
wp_update_https_detection_errors seems to be named what it is because it calls update_option() internally (which is an assumption on my part) and doesn't provide much benefit to include that detail in the function name, as the storage mechanism is not contextually relevant. It could be: wp_detect_https_errors, which is vague about its storage but assertive about what it does.
This was named based on other cron hook functions that update an option. wp_detect_https_errors
might be an option, but IMO it diverges from the existing naming and the name of the function sounds more like something a 3P developer would be welcome to use, which is not the case, since that cron hook is internal to core. wp_is_https_supported
is the "public" function to use.
wp_schedule_https_detection is following the pattern laid out by other wp_schedule_ functions, which are inconsistently named as well. update_network_counts, update_checks, delete_old_privace_export_files. Youch. Maybe wp_schedule_update_https_detection is better, even though it's not great?
The lack of a pattern here is a good point, though discussing this without deciding on a pattern blocks this ticket due to another problem. We could open a separate ticket to establish such a pattern, although arguably that would end up as something to contribute to the WP coding standards and best practices. Regarding your proposal, I don't think that is intuitive because it schedules the wp_https_detection
action, not the logic to update that option. Technically a pattern to potentially establish could be wp_schedule_{$hook_name}
(i.e. wp_schedule_wp_https_detection
), although that reads weird so maybe the pattern should be wp_schedule_{$hook_name_without_prefix}
(wp_schedule_https_detection
) - the latter is what's currently being used.
wp_is_owned_html_output is introducing a new global term related to content ownership. I'd like this to be reconsidered to avoid any possible future collisions with other APIs like roles & caps, multisite networks, etc... For example, #5942 wants to introduce an "Owner" role. If that happens, this function name would probably be undesirable. wp_is_local_html_output maybe? But "local" has similar connotations with development environments... Not sure about this one.
With this one, I agree there's room for improvement. I like the idea of "local" already better than "owned". For the site checking whether output comes from itself could be indicated by the "local" term - the same way like you could access your own server internally from "localhost" I guess?
These functions are also all marked as
@access private
but not prefixed with an underscore. Natch, this is broadly inconsistent, but has been enforced as recently as 5.5.0 with_wp_register_meta_args_allowed_list()
. Just want to make doubly sure this was considered and makes sense for these here.
This is something I years ago had a conversation about with @boonebgorges, and, while this is not documented anywhere (we definitely lack a best practices/patterns group!), he back then said that underscore-prefixed functions shouldn't be used anymore and that marking them with private is the guidance. I've stuck with that ever since - but I guess since it's not documented, not everyone is aware of that.
Let's keep discussing to see whether it's worth updating these function names and how. I guess we have 1-2 weeks to get this defined, but before RC these should be set in stone.
This ticket was mentioned in Slack in #core by flixos90. View the logs.
4 years ago
This ticket was mentioned in PR #948 on WordPress/wordpress-develop by felixarntz.
4 years ago
#37
This PR addresses follow-up feedback to https://core.trac.wordpress.org/changeset/49904 from https://github.com/WordPress/wordpress-develop/pull/870#issuecomment-763015259 (that PR is related, but targets another ticket).
It also renames a function as suggested by this comment.
Trac ticket: https://core.trac.wordpress.org/ticket/47577
adamsilverstein commented on PR #948:
4 years ago
#38
👍🏼 Overall code changes look great; i'll give this a test locally again as well.
TimothyBJacobs commented on PR #948:
4 years ago
#39
How do you feel about making wp_update_https_detection_errors
shortcircuitable? The reason I ask is I maintain a plugin that functions similarly, but uses an external server to ping the site to check for HTTPS to account for instances where a loopback request results in a false negative. Ideally, we'd be able to override the check to use the more accurate detection, but still keep the user in Site Health.
felixarntz commented on PR #948:
4 years ago
#40
@TimothyBJacobs Great suggestion - I'll add that tomorrow.
adamsilverstein commented on PR #948:
4 years ago
#41
@felixarntz I did some testing locally and took some screenshots. Noticed a couple of tiny issues:
Accessing wp-admin via http (with my invalid "self signed" SSL cert), I noticed an issue where the title is getting some extraneous encoding:
When I changed my WordPress Address to http I got a different message, looks good:
Site address only set to http, looks good:
Both set to http, switched to accessing wp-admin via http:
There is a slight problem here, the space is missing after the work "There":
felixarntz commented on PR #948:
4 years ago
#42
@adamsilverstein @TimothyBJacobs Addressed your feedback.
Here are some screenshots of the different states.
HTTPS not set up, not supported
HTTPS not set up, but supported
HTTPS not set up, but supported, with "WordPress Address" already using HTTPS though
HTTPS set up, but not supported (i.e. there are problems)
@TimothyBJacobs One question you might know the answer to: Now that I've made the test asynchronous and added a REST endpoint for it, do I need to update wp-api-generated.js
? At what point is that generated and how?
TimothyBJacobs commented on PR #948:
4 years ago
#43
@TimothyBJacobs One question you might know the answer to: Now that I've made the test asynchronous and added a REST endpoint for it, do I need to update
wp-api-generated.js
? At what point is that generated and how?
wp-api-generated.js
gets rebuild when the full test suite is run. I typically handle rebuilding it by running the unit tests in my svn checkout. But I also commit to the PR directly sometimes instead.
felixarntz commented on PR #948:
4 years ago
#45
Committed via https://core.trac.wordpress.org/changeset/50072.
This ticket was mentioned in PR #958 on WordPress/wordpress-develop by felixarntz.
4 years ago
#46
TimothyBJacobs commented on PR #958:
4 years ago
#47
Looks great to me!
felixarntz commented on PR #958:
4 years ago
#49
Committed via https://core.trac.wordpress.org/changeset/50075.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#51
follow-up:
↓ 52
@
4 years ago
If we are optimizing for https usage in 5.7, we could maybe add #49515
It seems that the hard work is already done in this ticket(s) here. The mentioned ticket just needs some UI/UX and copy decisions.
#52
in reply to:
↑ 51
@
4 years ago
Replying to zodiac1978:
If we are optimizing for https usage in 5.7, we could maybe add #49515
It seems that the hard work is already done in this ticket(s) here. The mentioned ticket just needs some UI/UX and copy decisions.
5.7 is now past the deadline (i.e. Beta 1) for all new features and enhancements. #49515 could be a possible 5.8 candidate.
#53
@
4 years ago
- Resolution set to fixed
- Status changed from reopened to closed
After discussing with @flixos90, closing the ticket as the enhancement work has been committed.
47577.diff makes the following changes:
wp_is_using_https()
, which detects whether the site uses HTTPS based on the WordPress site URL.wp_is_https_supported()
which detects whether HTTPS is supported by the environment (using a cached request to the HTTPS version of the site).