\Drupal\Core\Flood\MemoryBackend::isAllowed() does not work if no event registrations happened before:

$memory_flood->isAllowed('user.failed_login_ip', 50, 3600); // Exception: 'Undefined index: user.failed_login_ip'
CommentFileSizeAuthor
#11 2909952-9.patch2.1 KBcatch
#11 2909952-9-test-only.patch1.39 KBcatch
#9 interdiff-2909952-8-9.txt565 bytesedaa
#9 2909952-9.patch2.1 KBedaa
#8 interdiff-2909952-3-8.txt795 bytesedaa
#8 2909952-8.patch2.09 KBedaa
#7 interdiff-2910000-2-3.txt492 bytesedaa
#7 2909952-7.patch2.09 KBedaa
#3 interdiff-2909952-2-3.txt506 bytesedaa
#3 2909952-3.patch1.99 KBedaa
#2 2909952-2-failing-test.patch1.39 KBedaa
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

edwardaa created an issue. See original summary.

edaa’s picture

edaa CreditAttribution: edaa commented
Status: Active » Needs review
FileSize
2909952-2-failing-test.patch1.39 KB

Failing test.

edaa’s picture

edaa CreditAttribution: edaa commented
FileSize
2909952-3.patch1.99 KB
interdiff-2909952-2-3.txt506 bytes

New patch fixed the issue.

The last submitted patch, 2: 2909952-2-failing-test.patch, failed testing. View results

edaa’s picture

edaa CreditAttribution: edaa commented
Issue summary: View changes
edaa’s picture

edaa CreditAttribution: edaa commented
Related issues: +#2910000: Fix MemoryBackend::clearByPrefix with multiple items, and improve coverage and clarity

Add a related issue.

edaa’s picture

edaa CreditAttribution: edaa commented
FileSize
2909952-7.patch2.09 KB
interdiff-2910000-2-3.txt492 bytes

Take into account both the event name and user identifier.

edaa’s picture

edaa CreditAttribution: edaa commented
Title: Flood MemoryBackend::isAllowed() throws exception: 'Undefined index: user.failed_login_ip' » Flood MemoryBackend::isAllowed() throws exception: 'Undefined index: xxx'
FileSize
2909952-8.patch2.09 KB
interdiff-2909952-3-8.txt795 bytes

Fix #7 wrong interdiff file.

edaa’s picture

edaa CreditAttribution: edaa commented
FileSize
2909952-9.patch2.1 KB
interdiff-2909952-8-9.txt565 bytes

It's better to return $threshold > 0 than TRUE.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Needs review » Reviewed & tested by the community

Thank you @edwardaa! This fix totally makes sense for me. It is not allowed when you have a threshold of 0.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
FileSize
2909952-9-test-only.patch1.39 KB
2909952-9.patch2.1 KB

Uploading a test-only patch so we can see the test fails. Otherwise looks good but including a test-only patch helps things get committed quicker.

The last submitted patch, 11: 2909952-9-test-only.patch, failed testing. View results

  • catch committed 4e2d3ba on 8.5.x 
    Issue #2909952 by edwardaa: Flood MemoryBackend::isAllowed() throws...

  • catch committed c99427e on 8.4.x 
    Issue #2909952 by edwardaa: Flood MemoryBackend::isAllowed() throws...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented

Thank you catch!

Status: Fixed » Closed (fixed)

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