Warning: Undefined array key 1 in Drupal\autologout\EventSubscriber\AutologoutSubscriber->onRequest()

Created on 3 July 2023, 12 months ago
Updated 18 June 2024, 2 days ago

Warning: Undefined array key "destination" in Drupal\autologout\EventSubscriber\AutologoutSubscriber->onRequest() (line 137 of modules/contrib/autologout/src/EventSubscriber/AutologoutSubscriber.php).

Warning: Undefined array key 1 in Drupal\autologout\EventSubscriber\AutologoutSubscriber->onRequest() (line 148 of modules/contrib/autologout/src/EventSubscriber/AutologoutSubscriber.php)

๐Ÿ› Bug report
Status

RTBC

Version

2.0

Component

Code

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @sujan.shrestha
  • First commit to issue fork.
  • Merge request !34Fix: Undefined array key: destination โ†’ (Open) created by jigarius
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update 12 months ago
    15 pass, 7 fail
  • Status changed to Needs review 12 months ago
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada jigarius Montrรฉal

    Here's a PR that resolves the warning message. There seems to be a new concept of "destinations[]" now and I've not handled this for now.

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands ecvandenberg

    I tried the patch in a Drupal 9.5.10 site with autologout 8.1.4 and php 8.1.21.
    The warnings are gone.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom Chris--S

    This seems to be a duplicate of https://www.drupal.org/project/autologout/issues/3301642 ๐Ÿ› Throws Notice on Login when destination is set Needs work

  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine anprok

    Test failed. Need work.

  • ๐Ÿ‡ธ๐Ÿ‡ฎSlovenia DeaOm

    The tests that are failing seem to stem from a core issue ๐ŸŒฑ [META] Serialization issues in Migration tests Active . So either this issue can be blocked until the core issue is fixed, or based on the reviews that it's working can be merged. Leaving it up to maintainers and leaving the status as is.

  • ๐Ÿ‡ธ๐Ÿ‡ฎSlovenia DeaOm

    It also seems these are duplicates ๐Ÿ› Throws Notice on Login when destination is set Needs work and ๐Ÿ› Autologout does not handle multiple destinations Needs work .

  • Pipeline finished with Failed
    9 months ago
    Total: 336s
    #23384
  • Pipeline finished with Success
    9 months ago
    Total: 292s
    #23387
  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡ธ๐Ÿ‡ฎSlovenia DeaOm

    Corrected the CS issue for missing doc comment for the new getDestination function, testing is now done via GitLab CI, so test are passing now. Changing status to needs review so it can be re-tested.

  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 7.x + Environment: PHP 5.3 & MySQL 5.5
    last update 9 months ago
    Patch Failed to Apply
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sandipta

    Hi @DeaOm, your MR is failing. Please check

  • First commit to issue fork.
  • Pipeline finished with Success
    9 months ago
    Total: 348s
    #23418
  • Cleaned up the code a bit, since it's partially related to this issue, I think it's better to just fix it now. Otherwise, everything works as expected, and looking at the other two related issues, this one is cleaner and should probably be merged. Leaving the other two issues open so maintainers can credit them.

    Hiding the issue patch file, since the test there will always fail now that we are switching to GitlabCI. @sandipta this is probably why you thought that the MR was falling, it's not. Just integration with GitlabCI is different and doesn't display passing and failing tests the same way DrupalCI did.

    Leaving the issue to "needs review", because of the code changes I made.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sandipta

    Great. Thanks for the info @admirlju

  • Pipeline finished with Failed
    9 months ago
    Total: 348s
    #23703
  • Pipeline finished with Failed
    9 months ago
    Total: 318s
    #23708
  • Pipeline finished with Success
    9 months ago
    Total: 328s
    #23769
  • ๐Ÿ‡ธ๐Ÿ‡ฎSlovenia DeaOm

    The getDestination() function which was getting the destination from referrer was always returning an empty string, which means the if block was always skipped. It did solve the warning but also broke the functionality of redirecting user back to its own profile (if for example user/2 was logged in and then logged out and user/3 logged in after, the user/3 was redirected to the user/2 profile, because of the destination). Adjusted the condition to basically always redirect to its own user profile, so we do not need to explicitly check if user and user id match based on destination, getting ride of a chunk of code. Also added a test that checks that the destination is set and after user/2 is logged out and user/3 is logged in, the user/3 is redirected to the correct user profile. Ready for review.

  • Pipeline finished with Success
    9 months ago
    #24083
  • Tests were passing over gitlabCI but were not passing locally. This was down to /web/ being used in path checking for the destination query parameter. Changed it so it now dynamically gets the correct path depending on the Drupal configuration. Otherwise, the fix works, testing by hand if I get logged out on an account with UID 2 and login using an account with UID 3 it redirects me to the currently logged-in user profile.

    All that said, there is an issue if this should even be the functionality in the first place, for now, this should be merged since it fixes a bug in the current implementation of the module. But it would be nice if people contributed their opinions on this issue #3301938: Consider Backing out User Destination Manipulation (3101732) โ†’ .

    Because of the change to test, this still needs review.

  • Status changed to RTBC 9 months ago
  • ๐Ÿ‡ธ๐Ÿ‡ฎSlovenia DeaOm

    Tests are passing and I agree with the change to get the URL of user login page via route as this then guarantees tests are working everywhere.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium borisson_ Mechelen, ๐Ÿ‡ง๐Ÿ‡ช

    Can we copy over credit from ๐Ÿ› Throws Notice on Login when destination is set Needs work please?

  • First commit to issue fork.
  • Pipeline finished with Failed
    6 months ago
    #68480
  • Pipeline finished with Failed
    6 months ago
    #68481
  • ๐Ÿ‡ช๐Ÿ‡ธSpain unstatu

    I have applied a small change on the MR in order to improve the detection of the logic that should happen when destination parameter is found in the URL.

    Instead of using strpos (that is too generic), I have changed the implementation to use a regex.

    I have done that because I found a problem with the following situation:

    - Create a page with the following alias /user/my-user-info
    - Login with the following URL /user/login?destination=/user/my-user-info
    - After login, Drupal redirects to /user/ instead of /user/my-user-info because `strpos` detects the "user" word in the destination parameter

  • Status changed to Needs review 6 months ago
  • ๐Ÿ‡ช๐Ÿ‡ธSpain unstatu
  • i am getting the same warnings; hope there will be release with fix soon (:

  • ๐Ÿ‡ท๐Ÿ‡ดRomania claudiu.cristea Arad ๐Ÿ‡ท๐Ÿ‡ด

    claudiu.cristea โ†’ made their first commit to this issueโ€™s fork.

  • Status changed to RTBC 28 days ago
  • ๐Ÿ‡ท๐Ÿ‡ดRomania claudiu.cristea Arad ๐Ÿ‡ท๐Ÿ‡ด

    Rerolled after 1.5.0

  • Pipeline finished with Success
    28 days ago
    Total: 497s
    #180275
  • ๐Ÿ‡ท๐Ÿ‡ดRomania claudiu.cristea Arad ๐Ÿ‡ท๐Ÿ‡ด

    Moving to 2.x but I have no permission to change the base branch in MR

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pookmish

    Since it seems the MR patch url isn't working correctly (it seems outdated), here's a patch file that applies to both 1.x and 2.x branches. It's the same as the MR.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Krishna Mishra

    Patch is not working with 8.x-1.4, so generated new patch file (no logical change, only generated compatible patch) .
    Here is patch.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia jannakha Brisbane!

    +1

    please release

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada gwvoigt London, ON ๐Ÿ‡จ๐Ÿ‡ฆ

    Previous patches are not working with 8.x-1.5, so generated new patch file.

    also +1 to release this fix... thank you!

Production build 0.69.0 2024