Skip to content

Commit

Permalink
Fix error when a sitemap path starts with //
Browse files Browse the repository at this point in the history
When creating a Psr7 request with a base_uri set in the Client and a
path that starts with // (like '//wp-sitemap.xml'), the path is
interpreted as an absolute URL and we get a ClientException for
'unknown host: wp-sitemap.xml'. This is fixed by removing base_uri
from the client and always making Requests with the full url.

This was discovered while debugging #824 and may address that issue.
  • Loading branch information
john-shaffer committed Oct 9, 2021
1 parent 10ba290 commit bbc8abb
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- add ability to rewrite hosts specified on a new advanced options page @john-shaffer
- as part of this, changed the host replacement function to use strtr instead of str_replace to avoid replacing things that we just replaced
- add advanced option to skip URL rewriting @john-shaffer
- fix error when a sitemap path starts with // @jhatmaker, @john-shaffer

## WP2Static 7.1.7

Expand Down
5 changes: 2 additions & 3 deletions src/DetectSitemapsURLs.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public static function detect( string $wp_site_url ) : array {

$client = new Client(
[
'base_uri' => $base_uri,
'verify' => false,
'http_errors' => false,
'allow_redirects' => [
Expand Down Expand Up @@ -70,7 +69,7 @@ public static function detect( string $wp_site_url ) : array {
}
}

$request = new Request( 'GET', '/robots.txt', $headers );
$request = new Request( 'GET', $base_uri . '/robots.txt', $headers );

$response = $client->send( $request );

Expand Down Expand Up @@ -106,7 +105,7 @@ public static function detect( string $wp_site_url ) : array {
$sitemap
);

$request = new Request( 'GET', $sitemap, $headers );
$request = new Request( 'GET', $base_uri . $sitemap, $headers );

$response = $client->send( $request );

Expand Down

4 comments on commit bbc8abb

@jhatmaker
Copy link

Choose a reason for hiding this comment

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

John,

Thank you for the update. Unfortunately, this commit version still fails with the same symptoms as before.
I did verify on the system that the correct code was pushed out.

[11-Oct-2021 14:19:37 UTC] PHP Fatal error:  Uncaught WP2StaticGuzzleHttp\Exception\ClientException: Client error: `GET http://wplocal.internal//wp-sitemap.xml` resulted in a `404 Not Found` response:
<!doctype html>
<html lang="en-US" >
<head>
	<meta charset="UTF-8" />
	<meta name="viewport" content="width=device-width (truncated...)
 in /var/www/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticguzzle/src/Exception/RequestException.php:113
Stack trace:
#0 /var/www/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticguzzle/src/Middleware.php(69): WP2StaticGuzzleHttp\Exception\RequestException::create(Object(WP2StaticGuzzleHttp\Psr7\Request), Object(WP2StaticGuzzleHttp\Psr7\Response), NULL, Array, NULL)
#1 /var/www/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticpromises/src/Promise.php(204): WP2StaticGuzzleHttp\Middleware::WP2StaticGuzzleHttp\{closure}(Object(WP2StaticGuzzleHttp\Psr7\Response))
#2 /var/www/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticpromises/src/Promise.php(153): WP2StaticGuzzle in /var/www/html/wp-content/plugins/wp2static/src/DetectSitemapsURLs.php on line 132

@jhatmaker
Copy link

@jhatmaker jhatmaker commented on bbc8abb Oct 11, 2021

Choose a reason for hiding this comment

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

John,

Also note, I was able to duplicate the same issue with a fresh Lokl install and installing just the RankMath SEO WP plugin.
Then I removed the wp2static and installed the updated commit version and the problem/error was the same.

image

However, in looking at the error log on the system, it appears to be a different error on the Lokl instance.

[11-Oct-2021 15:13:07 UTC] PHP Fatal error:  Uncaught WP2StaticGuzzleHttp\Exception\TooManyRedirectsException: Will not follow more than 1 redirects in /usr/html/wp-conten
Stack trace:
#0 /usr/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticguzzle/src/RedirectMiddleware.php(88): WP2StaticGuzzleHttp\RedirectMiddleware->guardMax()
#1 /usr/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticguzzle/src/RedirectMiddleware.php(73): WP2StaticGuzzleHttp\RedirectMiddleware->checkRedirect()
#2 /usr/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticpromises/src/FulfilledPromise.php(41): WP2StaticGuzzleHttp\RedirectMiddleware->WP2StaticGuzzleHttp\{
#3 /usr/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticpromises/src/TaskQueue.php(48): WP2StaticGuzzleHttp\Promise\FulfilledPromise::WP2StaticGuzzleHttp\Pr
#4 /usr/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticpromises/src/Promise.php(248): WP2StaticGuzzleHttp\Promise\TaskQueue->run()
#5 /usr/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticpromises/src/Promise.php(224): WP2StaticGuzzleHttp\Promise\Promise->invokeWaitFn()
#6 /usr/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticpromises/src/Promise.php(62): WP2StaticGuzzleHttp\Promise\Promise->waitIfPending()
#7 /usr/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticguzzle/src/Client.php(123): WP2StaticGuzzleHttp\Promise\Promise->wait()
#8 /usr/html/wp-content/plugins/wp2static/src/Crawler.php(222): WP2StaticGuzzleHttp\Client->send()
#9 /usr/html/wp-content/plugins/wp2static/src/Crawler.php(110): WP2Static\Crawler->crawlURL()
#10 /usr/html/wp-content/plugins/wp2static/src/Crawler.php(71): WP2Static\Crawler->crawlSite()
#11 /usr/html/wp-includes/class-wp-hook.php(303): WP2Static\Crawler::wp2staticCrawl()
#12 /usr/html/wp-includes/class-wp-hook.php(327): WP_Hook->apply_filters()
#13 /usr/html/wp-includes/plugin.php(470): WP_Hook->do_action()
#14 /usr/html/wp-content/plugins/wp2static/src/Controller.php(821): do_action()
#15 /usr/html/wp-content/plugins/wp2static/src/Controller.php(702): WP2Static\Controller::wp2staticCrawl()
#16 /usr/html/wp-content/plugins/wp2static/src/Controller.php(811): WP2Static\Controller::wp2staticHeadless()
#17 /usr/html/wp-includes/class-wp-hook.php(301): WP2Static\Controller::wp2staticRun()
#18 /usr/html/wp-includes/class-wp-hook.php(327): WP_Hook->apply_filters()
#19 /usr/html/wp-includes/plugin.php(470): WP_Hook->do_action()
#20 /usr/html/wp-admin/admin-ajax.php(187): do_action()
#21 {main}
  thrown in /usr/html/wp-content/plugins/wp2static/vendor/leonstafford/wp2staticguzzle/src/RedirectMiddleware.php on line 147

This not be as relevant as just having the RankMath plugin activated produces that error (after deactivating the plugin's functionality after install), so the behavior is different than on my original test docker instance. I am doing some more testing on the lokl instance.

@bookwyrm
Copy link
Contributor

Choose a reason for hiding this comment

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

@jhatmaker I replicated the TooManyRedirects error and opened #826 with a fix.

Can you please try that code and see if you're still getting the original 500 error due to //? I'm unable to replicate that error in a fresh WP install on Lokl

@jhatmaker
Copy link

Choose a reason for hiding this comment

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

@bookwyrm - Thanks! I will give that a try. I did increment the max in the allow_redirects in the Crawler (from your commit/PR) which allowed the static site to be generated. But I will pull the full codebase again and run it through, but I suspect it is good.

Please sign in to comment.