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

Look at file_get_contents in sitemap class #1428

Closed
michaeltorbert opened this issue Dec 23, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@michaeltorbert
Copy link
Member

commented Dec 23, 2017

See if this can be improved in some way.

Some terrible hosts can potentially have a problem with seeking https://wordpress.org/support/topic/errors-creating-sitemap/#post-9808145

The problem is on a line with file_get_contents, and seems to be with the offset parameter. https://secure.php.net/manual/en/function.file-get-contents.php “Seeking (offset) is not supported with remote files. Attempting to seek on non-local files may work with small offsets, but this is unpredictable because it works on the buffered stream.”

@michaeltorbert michaeltorbert changed the title improve file_get_contents in sitemap class Look at file_get_contents in sitemap class Dec 23, 2017

@michaeltorbert

This comment has been minimized.

Copy link
Member Author

commented Dec 23, 2017

User reports error only comes on PHP 7.10, and not on either earlier or later versions of PHP

According to the changelog for PHP: "7.1.0 | Support for negative offsets has been added."

Our longtime use of the offsets parameter should be investigated, and why it's ONLY on exactly PHP 7.10.

@EkoJR EkoJR self-assigned this Dec 26, 2017

@EkoJR

This comment has been minimized.

Copy link
Member

commented Dec 26, 2017

I've been able to reproduce the issue, and investigated it a bit, and it seems the main issue is the difference between remote and local. Both files are local, but the compression prefix is causing PHP to recognize the string file path to be a remote file. The .xml file however works fine.

The issue also seems to be the cause of another issue, and happens that the warning function used also throws a PHP Warning since 7.1.0+.

Callstack

#	Time	Memory	    Function	                                           Location
1	0.0000	459168          {main}( )                                              ...\admin.php:0
2	0.4280	32825256	do_action( )                                           ...\admin.php:224
3	0.4280	32825632	WP_Hook->do_action( )                                  ...\plugin.php:453
4	0.4280	32825632	WP_Hook->apply_filters( )                              ...\class-wp-hook.php:310
5	0.4280	32826648	All_in_One_SEO_Pack_Sitemap->display_settings_page( )  ...\class-wp-hook.php:286
6	3.9022	42739440	do_action( )                                           ...\aioseop_module_class.php:2591
7	3.9022	42739816	WP_Hook->do_action( )                                  ...\plugin.php:453
8	3.9022	42739816	WP_Hook->apply_filters( )                              ...\class-wp-hook.php:310
9	3.9022	42740808	All_in_One_SEO_Pack_Sitemap->do_sitemap_scan( )        ...\class-wp-hook.php:286
10	3.9022	42740808	All_in_One_SEO_Pack_Sitemap->scan_sitemaps( )          ...\aioseop_sitemap.php:965
11	3.9132	42741320	All_in_One_SEO_Pack_Sitemap->sitemap_warning( )        ...\aioseop_sitemap.php:982
12	3.9132	42741344	All_in_One_SEO_Pack_Sitemap->get_problem_files( )      ...\aioseop_sitemap.php:1083
13	3.9212	42759152	file_get_contents ( )                                  ...\aioseop_sitemap.php:1034

Reproduce

This issue also affects other versions from 7.1.0 to 7.2.0 (current), and can be reproduced fairly easy. To reproduce...

  1. Go to XML Sitemap settings, and under the XML Sitemap metabox.
  2. Check "Create Compressed Sitemap".
  3. Un-check "Dynamically Generate Sitemap".
  4. Saving options by clicking "Update Sitemap" button will trigger the bug.

Solution

I was thinking of handling the compress file separately with PHP's gzfile() for this section of code, but the initial problem seems to be handled differently using WP's filesystem object; which may need to be investigated further.

@EkoJR

This comment has been minimized.

Copy link
Member

commented Dec 28, 2017

Reproduce

Additional environmental conditions that may contribute to bug...

  • Unknown Rewrite issue. Was possibly the main contributor to the PHP error message itself being triggered, but was not the root issue. In my case, it was the localhost URL that I suspected.

7.1.0+ PHP Development

It may already be assumed that this was an intended change with PHP, and when investigating the matter, it may lead someone to believe that this is a PHP bug; there are some mentions of 7.1+ issues related to the new forced rule. So this may or may not be a PHP bug.

The reason for the change was due to an exploit that could allow hackers to get into a server through a remote file being used. However, AIOSEOP files are locally stored, but the issue is with the compress streams in file_get_contents(). Which makes it appear as a bug, or a feature/concept that hasn't been added to Compression Streams yet; possibly being fixed/added in the future

As of AIOSEOP, the equivalent would be gzopen(), gzread(), and gzclose(), and would be more manually controlled, but does add some extra lines of code.

Commit

Compress files are handled separately to prevent conflicts, and the offset for files_get_content() was changed from -1 to 0 to fix what lead the sitemap not being recognized, and possibly deleted.

A note/todo was added which identifies a possible flaw; may be partially intended.

Additional Resources

EkoJR added a commit to EkoJR/all-in-one-seo-pack that referenced this issue Dec 28, 2017

Fix PHP Error & Sitemap not generating semperfiwebdesign#1428
* Issue#1428 - Fix PHP Error and Sitemap not generating.
    * Fix AIOSEOP Error message "Removed empty file *" with Static Compressed Sitemap being deleted automatically because of a false-false return.
    * Fix PHP 7.1.0 - 7.2.0+ (current) Warning with Compressed Streams to file path.
* Fix Static Sitemap undetected to delete when Dynamic Sitemap is enabled.
* Fix Error message "Potential conflict with unknown file *" with a valid .xml file.

@EkoJR EkoJR added Bug Needs Testing and removed Initial Review labels Jan 3, 2018

@EkoJR EkoJR added this to the 2.4.4 milestone Jan 3, 2018

@wpsmort wpsmort modified the milestones: 2.4.4, 2.4.5 Jan 11, 2018

@michaeltorbert michaeltorbert modified the milestones: 2.4.5, 2.4.6 Jan 31, 2018

@wpsmort

This comment has been minimized.

Copy link
Member

commented Feb 5, 2018

@EkoJR Is there a PR for this?

@michaeltorbert

This comment has been minimized.

Copy link
Member Author

commented Feb 7, 2018

@wpsmort It looks like #1435 is referencing this issue. PRs are the ones with the curved arrow symbol instead of the exclamation point inside a circle.

@wpsmort wpsmort self-assigned this Feb 7, 2018

@michaeltorbert michaeltorbert modified the milestones: 2.4.6, 2.4.7 Mar 15, 2018

@wpsmort wpsmort modified the milestones: 2.4.7, 2.5 Mar 20, 2018

@wpsmort wpsmort removed the Needs Testing label Mar 22, 2018

michaeltorbert added a commit that referenced this issue Mar 22, 2018

Fix PHP Error & Sitemap not generating #1428 (#1435)
* Issue#1428 - Fix PHP Error and Sitemap not generating.
    * Fix AIOSEOP Error message "Removed empty file *" with Static Compressed Sitemap being deleted automatically because of a false-false return.
    * Fix PHP 7.1.0 - 7.2.0+ (current) Warning with Compressed Streams to file path.
* Fix Static Sitemap undetected to delete when Dynamic Sitemap is enabled.
* Fix Error message "Potential conflict with unknown file *" with a valid .xml file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.