Skip to content

Conversation

@spinsch
Copy link
Contributor

@spinsch spinsch commented Oct 9, 2020

Description

In most cases it will never be a problem, but in test environments with the highest error level it will not work.
list() wants numeric arrays

Manual testing scenarios

  1. Visit a page

Questions or comments

Prerequisite

  • Run php with error_reporting = E_ALL
  • Configure redis as session backend

ENV

  • PHP 7.3.22
  • Redis 6.05

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

@github-actions github-actions bot added the Component: Cm/RedisSession Relates to Cm_RedisSession label Oct 9, 2020
@joshua-bn
Copy link
Contributor

joshua-bn commented Oct 9, 2020

Weird. I'm on 7.4 with E_ALL and don't have this issue.

@Serjudo
Copy link

Serjudo commented Oct 20, 2020

In my case with PHP 7.2, I have the following errors in system.log

2020-10-20T16:22:09+02:00 ERR (3): Notice: Undefined offset: 0 in /home/wwwpapelstore/public_html/lib/Cm/Cache/Backend/Redis.php on line 799

2020-10-20T16:22:09+02:00 ERR (3): Notice: Undefined offset: 1 in /home/wwwpapelstore/public_html/lib/Cm/Cache/Backend/Redis.php on line 799

2020-10-20T16:22:09+02:00 ERR (3): Notice: Undefined offset: 2 in /home/wwwpapelstore/public_html/lib/Cm/Cache/Backend/Redis.php on line 799

@Flyingmana
Copy link
Contributor

Described more in depth in the related PR #1705
As this is a repeating issue/fix now, I count this as one approved Review.

@Flyingmana Flyingmana merged commit 9b425f4 into OpenMage:1.9.4.x Jul 6, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2021

Unit Test Results

1 files  1 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌
6 runs  4 ✔️ 2 💤 0 ❌

Results for commit 9b425f4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Cm/RedisSession Relates to Cm_RedisSession

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants