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

can't parse_url() for creating URIs that use pagination logic #6331

Closed
samyouel opened this issue Mar 6, 2024 · 4 comments · Fixed by #6560
Closed

can't parse_url() for creating URIs that use pagination logic #6331

samyouel opened this issue Mar 6, 2024 · 4 comments · Fixed by #6560
Assignees
Labels
type: bug 🐛 Is a bug; fixes a bug
Milestone

Comments

@samyouel
Copy link

samyouel commented Mar 6, 2024

Description

I've been following the cookbook to setup 'Load More' Functionality with Kirby Pagination.

The model named 'Rewards' is using virtual pages, to fetch the children of the model from an API.

I'm using page cache to cache the content of the page.
During the function children(), when I call the children of the model over API, I clear the page cache by using kirby()->cache('pages')->remove('rewards.html') in certain conditions.

This accessing the page cache is not working once I click the 'Load More' Button, that calls the paginated children from the rewards.json.php.

Expected behavior

The 'Load More' Button should fetch the children in the same way and display them, as an initial page load should be able to as well.

Actual behaviour

When I use the AJAX 'Load More' Button as outlined in the cookbook, the request fails upon the moment of accessing the page cache.

I've traced it down to kirby/src/Http/Uri.php:91, during accessing the page cache, it tries to build a URI, but somehow the parse_url() function from php can't build a url from the pagination url "/rewards.json/page:2" that is being sent with the call.
I guess because of the colon :.

Stack trace

"NOTICE: PHP message: Whoops\Exception\ErrorException: Automatic conversion of false to array is deprecated in /var/www/html/kirby/src/Http/Uri.php:92"
"Stack trace:"
"#0 /var/www/html/kirby/src/Http/Uri.php(92): Whoops\Run->handleError(8192, 'Automatic conve...', '/var/www/html/k...', 92)"
"#1 /var/www/html/kirby/src/Http/Uri.php(214): Kirby\Http\Uri->__construct(Array, Array)"
"#2 /var/www/html/kirby/src/Cms/App.php(1662): Kirby\Http\Uri::current(Array)"
"#3 /var/www/html/kirby/src/Cms/System.php(159): Kirby\Cms\App->url('index', true)"
"#4 /var/www/html/kirby/src/Cms/AppCaches.php(80): Kirby\Cms\System->indexUrl()"
"#5 /var/www/html/kirby/src/Cms/AppCaches.php(32): Kirby\Cms\App->cacheOptions('pages')"
"#6 /var/www/html/site/models/rewards.php(29): Kirby\Cms\App->cache('pages')"
"#7 /var/www/html/site/controllers/rewards.json.php(6): RewardsPage->children()"
"#8 [internal function]: Kirby\Cms\App->{closure}(Object(RewardsPage))"
"#9 /var/www/html/kirby/src/Toolkit/Controller.php(60): Closure->call(Object(Kirby\Cms\App), page..."

To reproduce

  1. try parse_url('/rewards.json/page:2') that is used in the mentioned file
  2. it will give you false, because the URL is malformed.

-> Is there something wrong with this URI Constructor?

Your setup

Kirby Version
getkirby/cms (4.1.1) composer install

@distantnative distantnative added the type: bug 🐛 Is a bug; fixes a bug label Mar 23, 2024
@distantnative
Copy link
Member

distantnative commented Mar 23, 2024

Summary

Pinging @bastianallgeier @lukasbestle for ideas as you have a better insight on the Http classes, I think.

@lukasbestle
Copy link
Member

Am I correct that this only happens if Kirby's url option was configured without a host (e.g. as 'url' => '/')?

But the underlying bug seems to be in Uri::current() or actually in the Uri constructor. Maybe we could solve it there by adding a fake host for non-absolute URLs and removing the injected data from the array that parse_url() returns again. We do a similar thing in the Environment class:

// make sure the URL parser works properly when there's a
// colon in the request URI but the URI is relative
if (Url::isAbsolute($requestUri) === false) {
$requestUri = 'https://getkirby.com' . $requestUri;
}

@distantnative
Copy link
Member

Those lines form the Environment class sound like exactly what's needed here as well.

@jensscherbl
Copy link

jensscherbl commented May 17, 2024

This sounds kinda familiar. Not sure if it’s really helpful here, but I fixed a similar issue in the library that I use for my HTML minification plugin a while ago. See hexydec/htmldoc#8

Am I correct that this only happens if Kirby's url option was configured without a host (e.g. as 'url' => '/')?

Exactly. As noted on php.net regarding parse_url():

This function may not give correct results for relative or invalid URLs [...]

and

This function is intended specifically for the purpose of parsing URLs and not URIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Is a bug; fixes a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants