Problem/Motivation

\Drupal\user\Plugin\EntityReferenceSelection\UserSelection::entityQueryAlter() does a str_replace() on sql without accounting for the underlying escaping strategy.

Proposed resolution

Use escapeField()

This is not testable but if all identifiers are quoted as per #2986452: Database reserved keywords need to be quoted as per the ANSI standard then this will become a problem. It already is a problem for contrib or custom db drivers that do anything interesting in escapeField().

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 2986539-2.patch1.12 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
Related issues: +#2986452: Database reserved keywords need to be quoted as per the ANSI standard
StatusFileSize
new 2986539-2.patch1.12 KB
amateescu’s picture

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

Looks good to me :)

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Don't we need tests?

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

This is not testable

nvm #4.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Also just for clarification - we do have existing tests that prove the implementation is not broken by this change. \Drupal\Tests\system\Functional\Entity\EntityReferenceSelection\EntityReferenceSelectionAccessTest::testUserHandler() has test coverage for the case when anonymous users are included or excluded.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2986539-2.patch, failed testing. View results

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Reviewed & tested by the community

  • catch committed 0f02ad4 on 8.7.x 
    Issue #2986539 by alexpott: \Drupal\user\Plugin\EntityReferenceSelection...
catch’s picture

catch
he/him
Primary language English
commented
Version: 8.7.x-dev » 8.6.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.7.x and cherry-picked to 8.6.x. Thanks!

  • catch committed 05dac1d on 8.6.x 
    Issue #2986539 by alexpott: \Drupal\user\Plugin\EntityReferenceSelection...

Status: Fixed » Closed (fixed)

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