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

Auto-Cache Engine forces non-SSL scheme for XML Sitemap URL #715

Closed
raamdev opened this issue Mar 14, 2016 · 8 comments
Closed

Auto-Cache Engine forces non-SSL scheme for XML Sitemap URL #715

raamdev opened this issue Mar 14, 2016 · 8 comments

Comments

@raamdev
Copy link
Contributor

raamdev commented Mar 14, 2016

The Auto-Cache Engine forces an http scheme when building the Site URL for the sitemap that the Auto-Cache Engine uses:
https://github.com/websharks/comet-cache-pro/blob/160227/src/includes/classes/AutoCache.php#L89-L95

It then uses that (forced http) URL when fetching the sitemap:
https://github.com/websharks/comet-cache-pro/blob/160227/src/includes/classes/AutoCache.php#L100

This is likely to cause problems with sites that are HTTPS-only and which are using certain methods of enforcing HTTPS (e.g., certain SSL plugins or rewriting techniques). I'm not seeing any reason we should be forcing http here.


Referencing internal ticket: https://websharks.zendesk.com/agent/tickets/11700

@raamdev
Copy link
Contributor Author

raamdev commented Mar 14, 2016

@jaswsinc Any feedback here? You wrote this part of the code and I don't see any code comments indicating a reason http is forced here, so I'm wondering if you had something specific in mind when you wrote this.

@jaswrks
Copy link

jaswrks commented Mar 23, 2016

I agree there is room for improvement here.

The reason I'm forcing http is because network_home_url() and home_url() will use the current scheme by default, which may or may not be desirable given the context of a CRON job that is running behind-the-scenes.

One way to improve this would be to force a scheme that matches their WordPress URL in the config options; i.e., $scheme = parse_url(get_option('home_url'), PHP_URL_SCHEME);


Other Thoughts...

This report of a problem also suggests there might be a problem with this method. It is configured to follow up to 5 redirects, which should be enough for the web server to tell us which scheme it prefers and for that test to succeed. However, this report suggests that method could be failing in some scenarios.

In short, using http as a default scheme for a sitemap should work regardless, and our test for the existence of a sitemap should be capable of following any redirects that a web server issues in order to force a specific scheme that a given web server prefers.

  • https may or may not work on some web servers.
  • http should always work on some level, and if it is incorrect a redirect should be followed.

@raamdev
Copy link
Contributor Author

raamdev commented Mar 23, 2016

which may or may not be desirable given the context of a CRON job that is running behind-the-scenes.

Not desirable in what way?

http should always work on some level, and if it is incorrect a redirect should be followed.

That seems like a pretty bad assumption, especially given the trend towards HTTPS everywhere.

@raamdev
Copy link
Contributor Author

raamdev commented Mar 23, 2016

https may or may not work on some web servers.

Right, but if a site owner has configured their network_home_url() or home_url() with an HTTPS address, why shouldn't it be safe to assume that https works?

@raamdev
Copy link
Contributor Author

raamdev commented Mar 23, 2016

Not desirable in what way?

Oh, I see. network_home_url() and home_url() set the scheme based on is_ssl(), which just checks if the page is using SSL.

One way to improve this would be to force a scheme that matches their WordPress URL in the config options; i.e., $scheme = parse_url(get_option('home_url'), PHP_URL_SCHEME);

Agreed. That makes a lot more sense.

@jaswrks
Copy link

jaswrks commented Mar 23, 2016

Agreed. That makes a lot more sense.

Cool.

That seems like a pretty bad assumption, especially given the trend towards HTTPS everywhere.

Well, http:// is still a default scheme for HTTP. So even as sites move to pure https:// throughout, they must all still accept http:// traffic in some form; e.g., if you simply type www.example.com into a browser, the browser will contact the site over http:// first, and then it's the web server's job to upgrade the connection to https:// if that is what it prefers.

@raamdev
Copy link
Contributor Author

raamdev commented Apr 6, 2016

Next Release Changelog:

  • Enhancement (Pro): Improved the way the Auto-Cache Engine figures out the URL scheme (http vs https) that should be used when fetching the XML Sitemap. Instead of forcing http, whatever scheme is configured with the Home URL is now used. See Issue #715.

@raamdev raamdev closed this as completed Apr 6, 2016
@raamdev raamdev added enhancement and removed bug labels Apr 6, 2016
@raamdev
Copy link
Contributor Author

raamdev commented Apr 16, 2016

Comet Cache v160416 has been released and includes changes from this GitHub Issue. See the v160416 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#715).

@wpsharks wpsharks locked and limited conversation to collaborators Apr 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants