Problem/Motivation

With a views pager set to 'Allow user to display all items' in the 'Exposed Options' section, along with setting the exposed items per page options set, when viewing the page if the user selects '- All -' from the Items per page select list, this causes a white screen unexpected error to appear when the filter is applied. The error message is below:

This only happens with PHP 8. I can reproduce this behaviour on multiple sites. Tried to test it with simplytest.me, but looks like that is still running only PHP 7.3 so does not error.

TypeError: Unsupported operand types: int * string in Drupal\views\Plugin\views\pager\SqlBase->query() (line 279 of /var/www/html/core/modules/views/src/Plugin/views/pager/SqlBase.php)
#0 /var/www/html/core/modules/views/src/Plugin/views/pager/Mini.php(48): Drupal\views\Plugin\views\pager\SqlBase->query()
#1 /var/www/html/core/modules/views/src/Plugin/views/query/Sql.php(1457): Drupal\views\Plugin\views\pager\Mini->query()
#2 /var/www/html/core/modules/views/src/ViewExecutable.php(1321): Drupal\views\Plugin\views\query\Sql->build(Object(Drupal\views\ViewExecutable))
#3 /var/www/html/core/modules/views/src/Plugin/views/display/PathPluginBase.php(395): Drupal\views\ViewExecutable->build()
#4 /var/www/html/core/modules/views/src/Plugin/views/display/Page.php(196): Drupal\views\Plugin\views\display\PathPluginBase->execute()
#5 /var/www/html/core/modules/views/src/ViewExecutable.php(1630): Drupal\views\Plugin\views\display\Page->execute()
#6 /var/www/html/core/modules/views/src/Element/View.php(81): Drupal\views\ViewExecutable->executeDisplay('page_1', Array)
#7 [internal function]: Drupal\views\Element\View::preRenderViewElement(Array)
#8 /var/www/html/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php(101): call_user_func_array(Array, Array)
#9 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(772): Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_ren...', 'exception', 'Drupal\\Core\\Ren...')
#10 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(363): Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array)
#11 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(201): Drupal\Core\Render\Renderer->doRender(Array, false)
#12 /var/www/html/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(241): Drupal\Core\Render\Renderer->render(Array, false)
#13 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(564): Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}()
#14 /var/www/html/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(242): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#15 /var/www/html/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(132): Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch))
#16 /var/www/html/core/lib/Drupal/Core/EventSubscriber/MainContentViewSubscriber.php(90): Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch))
#17 [internal function]: Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#18 /var/www/html/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(142): call_user_func(Array, Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#19 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(163): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view')
#20 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(80): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#21 /var/www/html/core/lib/Drupal/Core/StackMiddleware/Session.php(58): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#22 /var/www/html/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#23 /var/www/html/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#24 /var/www/html/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#25 /var/www/html/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#26 /var/www/html/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#27 /var/www/html/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#28 /var/www/html/core/lib/Drupal/Core/DrupalKernel.php(708): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#29 /var/www/html/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#30 {main}

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#26 3266341-26.patch717 bytesAnkit.Gupta
#9 3266341-9.patch676 bytescarsteng
#4 3266341-4.patch3.58 KBcilefen

Issue fork drupal-3266341

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

david.qdoscc created an issue. See original summary.

cilefen’s picture

cilefen commented
Priority: Normal » Major
Issue summary: View changes

It is reproducible and as it's an exception you can cause in the UI, it's major priority. I updated the IS with a stack trace.

cilefen’s picture

cilefen commented

It is of course caused by loose typing. But the problem goes deep. If you do some typecasting in SqlBase::query, you then have to backtrack through the calling code because of the same problem.

cilefen’s picture

cilefen commented
Version: 9.3.x-dev » 9.4.x-dev
Status: Active » Needs review
StatusFileSize
new 3266341-4.patch3.58 KB

There is actually a lot to be done. This fixes it for the mini-pager (I think). It's just a first pass.

cilefen’s picture

cilefen commented
Status: Needs review » Needs work

#4 definitely doesn't work. There is no exception, but you only get one item for selecting '--All--'.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue tags: +Needs tests

I don't want to have to add this tag as I know how difficult Views tests can be, but this is a case where we really need some more.

cilefen’s picture

cilefen commented
Title: Views pager display all items PHP 8.0 error » Views pagers do math on disparate data types, resulting in type errors in PHP 8
carsteng’s picture

carsteng commented

Path in #4 is not good.....
The root problem here is that with Line 263 in SqlBase.php with php8.0 returns true.

$items_per_page = 'All';

# php8.0
($items_per_page > 0) -> TRUE

# php7.4
($items_per_page > 0) -> FALSE
carsteng’s picture

carsteng commented
StatusFileSize
new 3266341-9.patch676 bytes

And here is the patch:

carsteng’s picture

carsteng commented
Status: Needs work » Needs review
david.qdoscc’s picture

david.qdoscc commented

@carstenG thank you for the fantastically simple fix! Patch #9 does the job :)

cilefen’s picture

cilefen commented
Status: Needs review » Needs work

Alright let's add the test coverage.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

fkildoo’s picture

fkildoo commented

Patch #9 fixed the issue on my end.

freddy rodriguez’s picture

freddy rodriguez
Location Bogotá
commented

There is a kind of strange glitch: With the patch #9 applied, the error appears only right after saving the display. If the user re-enters the view and saves, everything seems to work normally.

The patch#9 was tested in D9.4.3 - PHP 8.0.21

jshimota01’s picture

jshimota01 commented

tested #9 successfully - php 8.1.6, d9.4.5 - had to clear Views Cache

brewerkr’s picture

brewerkr commented

when I try #9, I just get a blank page. Drupal 9.4.5 PHP 8.0.20. I was getting the same when I was at D9.4.3 (thus the upgrade to 9.4.5 with fingers crossed). Also tried using PHP 8.1 with same result (but got deprecation errors for another module so went back to PHP 8.0)

davidhk’s picture

davidhk commented

I applied the patch, cleared all caches, and now the 'All' value of items-per-page works again. I'm using Drupal 9.4.5 with PHP 8.0.23.

nlisgo’s picture

nlisgo commented
Issue tags: +Drupalcon Prague 2022

I'm going to attempt to add a test for this.

nlisgo’s picture

nlisgo commented
Issue tags: -Drupalcon Prague 2022 +Prague2022

nlisgo opened merge request !2791

nlisgo’s picture

nlisgo commented

I've created a merge request initially adding a test that will expose the error described in this issue to CI. Once this fails I will add a commit which applies the patch in #9 and CI should then pass.

nlisgo opened merge request !2793

nlisgo’s picture

nlisgo commented

After chatting with @xjm I have decided it is best to have 2 merge requests. One which has the test and patch and the other which is test only.

- test and fix
- test only, expected to fail

nlisgo’s picture

nlisgo commented
Status: Needs work » Needs review
Ankit.Gupta’s picture

Ankit.Gupta commented
StatusFileSize
new 3266341-26.patch717 bytes

Rerolled patch #9 with Drupal 9.5.x

Ankit.Gupta’s picture

Ankit.Gupta commented

Needs Review

nlisgo’s picture

nlisgo commented

Test only merge request !2793 failed as expected.

Merge request !2791 needs a review.

philltran’s picture

philltran commented

Reviewed and tested on a Drupal 9.4.x site. The error message is no longer appearing in my watchdog. Thanks.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

puregin’s picture

puregin commented

Patch #26 tested on a Drupal 9.5.x site (PHP 8.1.12). The view renders correctly and the error is no longer being thrown.

cilefen’s picture

cilefen commented
Issue tags: +PHP 8.0

Someone please write a regression test.

poker10’s picture

poker10 commented

Actually a simple test seems to be already created in the merge request, see #28. However it is not contained in the patch file rerolled in #26.

ppound’s picture

ppound commented

I believe there are other variables that can be set to "All" at least when using a list field type, around line 752 of ./views/src/Plugin/views/field/EntityField.php
$delta_limit = $this->options['delta_limit']; can return "All" and then create white screens. This can probably be fixed around line 412 of the same file

arturopanetta’s picture

arturopanetta
Primary language Italian
Location Grotteria (RC)
commented

I tested patch #26 on Drupal 9.5.1 and PHP 8.1, it works perfectly.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests +Needs Review Queue Initiative

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Hiding the patch in #26 as it skipped over the MRs

MR 2793 = tests-only
MR 2791 = full patch

Removing tests tag as they were in added in the MR

On Drupal 10.1 with php8.1 on a standard install
I followed the steps in the IS and confirmed the issue still appears
MR 2791 still applies to 10.1
Problem is solved.

quietone’s picture

quietone commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review

I don't see an answer to the question in #34? Setting back to NR for that.

I have also added the IS template, and invite everyone to complete that to help reviewers and committers. Thanks.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

edit: whoops that was a bit too fast!

Updated the IS with steps to reproduce, the issue in #34 is unrelated and needs a new issue with it's own steps to reproduce.

The test looks good, would kinda prefer to a have a functional test for this too, since it's a pretty common scenario to do this and I'm a bit sad that we don't have coverage that would cover this scenario. The unit test hits the right code path for this specific bug, but it would be nice if we had broader coverage for this feature

danchadwick’s picture

danchadwick commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review

TL;DR: Let's commit this, then open new issue(s) for similar situations.

This issue is the poster child for agility.

- Drupal now requires PHP 8+.
- PHP 8+ causes this bug.
- This bug has a nasty failure mode -- white screen of death.
- This bug occurs frequently; lots of sites are affected by this common configuration.
- The fix is both trivial and low-risk, only restoring the pre-8.0 meaning of the code.
- The fix was posted a year ago.
- Took a year to satisfy the demand for automated tests.
- Finally marked RTBC
- Now issue creep puts it back to Needs Review, expanding the issue beyond the original scope reported and including less frequent, less urgent situations.

If medicine worked this way, we'd still be waiting for the C19 vaccines.

danchadwick’s picture

danchadwick commented
Status: Needs review » Reviewed & tested by the community

Whoops. My post crossed with #38. Restoring status to RTBC as per #38.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented

Opened new issue for #34, very easy to reproduce, good find, see #3346748: Entering a non-numeric value for a start row value in 'Multiple field settings' for a views field leads to a fatal error

catch’s picture

catch
he/him
Primary language English
commented
Version: 10.1.x-dev » 9.5.x-dev
Priority: Major » Critical
Status: Reviewed & tested by the community » Fixed

Retrospectively bumping this to critical since it's a fatal error in common configurations.

Committed cbd7cf7 and pushed to 10.1.x. Thanks! also backported to 10.0.x and 9.5.x

  • catch committed da1de2ff on 10.0.x 
    Issue #3266341 by nlisgo, cilefen, carstenG, david.qdoscc, Lendude,...

  • catch committed 84547a79 on 10.1.x 
    Issue #3266341 by nlisgo, cilefen, carstenG, david.qdoscc, Lendude,...

  • catch committed ab3916cf on 9.5.x 
    Issue #3266341 by nlisgo, cilefen, carstenG, david.qdoscc, Lendude,...
catch’s picture

catch
he/him
Primary language English
commented

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.