Problem/Motivation

When performing in_array() comparisons on strings, especially in security/access components - we should pass TRUE to the third parameter of in_array() to ensure a strict (===) comparison.

Proposed resolution

Do it.

Remaining tasks

Review

User interface changes

None

API changes

None

CommentFileSizeAuthor
#7 node-access-strict-2337227-7.patch1.18 KBer.pushpinderrana
#1 node-access-strict-2337227.1.patch1.14 KBlarowlan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Active » Needs review
FileSize
node-access-strict-2337227.1.patch1.14 KB
chx’s picture

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

While the chances of calling a field "0" is not high... let's not open that door.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Yeah, field names are *supposed* to be lowercase, but we only validate that for configurable fields at the moment.

I doubt it would work to have a field named Bla and one bla, so doesn't make much difference practically, but fine with me ;)

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented

Could you please delay committing this patch until #2287071: Add cacheability metadata to access checks is committed? That ~300K patch is very, very painful to reroll and this one will conflict. Sorry, and thanks for your consideration. I will help with rerolling this patch if you like.

catch queued 1: node-access-strict-2337227.1.patch for re-testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

#2287071: Add cacheability metadata to access checks landed

er.pushpinderrana’s picture

er.pushpinderrana CreditAttribution: er.pushpinderrana commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
node-access-strict-2337227-7.patch1.18 KB

Rerolled #1 patch.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Status: Needs review » Reviewed & tested by the community

  • alexpott committed e07bf5f on 8.0.x 
    Issue #2337227 by er.pushpinderrana, larowlan: Fixed...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed e07bf5f and pushed to 8.0.x. Thanks!

Status: Fixed » Closed (fixed)

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