Problem/Motivation

When scanning the captcha module, we appear to be getting a false positive due to the captcha type token looks like a library.

Eg:

web/modules/contrib/captcha/tests/src/Functional/CaptchaCacheTest.php 	86 	
The 'captcha_test/TestCacheable' library is not defined because the defining extension is not installed. Cannot decide if it is deprecated or not.

However this is not a library at all.

Steps to reproduce

Download latest version of captcha, run upgrade status.

Proposed resolution

Update the library token to not simply look at the 'module_name/library'.

Comments

japerry created an issue.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

What's the PHP code this is tokenized out of?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Acquia commented

https://git.drupalcode.org/project/captcha/-/blob/60078bf45942692c1ad8bf...

    // Enable captcha on login block with a cacheable captcha.
    $type = 'captcha_test/TestCacheable';
    captcha_set_form_id_setting('user_login_form', $type);

Looks like it catches anything in the form of valid_extension_name/%

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

Since it needs to cache removed libraries, how else would we solve this? :) Here is the code: https://git.drupalcode.org/project/upgrade_status/-/blob/8.x-3.x/src/Lib...

We could definitely update the error message to say this may be a false positive?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Acquia commented

I don't have a great answer. I thought about only checking if the value was used in an array, but that may not be correct.

It can be worked around using string concatenation to work around limitations of token_get_all

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

A related issue at #3304830: Library analyzer is not ignoring fixture files was resolved, but I think there will still be false positives in the above format that we can't avoid. My position is that Upgrade Status is not entirely foolproof and human reviews are always required at the end.