Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
If a module contains an empty libraries.yml file, LibraryDiscoveryParser::parseLibraryInfo() passes NULL to hook_library_info_alter implementations. If any implementation type hints the $libraries parameter as an array, a fatal error occurs:
TypeError: Argument 1 passed to locale_library_info_alter() must be of the type array, null given, called in web/core/lib/Drupal/Core/Extension/ModuleHandler.php on line 539 in locale_library_info_alter() (line 592 of core/modules/locale/locale.module).
Since library definitions are normally only loaded when a library from the module is attached to the page, this error may not occur in normal operation. However, the Content Security Policy Module needs to enumerate all library definitions for a site, resulting in files that wouldn't otherwise be accessed getting parsed.
Steps to reproduce
- Create an empty
libraries.ymlfile in a module - Enable the locale module, which type hints
locale_library_info_alter() - Enable csp.module and clear caches
- Load any page on the site
Proposed resolution
Check that the result of parsing libraries.yml file is not NULL
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | Screenshot from 2020-08-31 17-56-04.png | 240.45 KB | thalles |
| #13 | drupal-3167036-13.patch | 2.17 KB | gapple |
| #13 | drupal-3167036-13-test-only.patch | 1.39 KB | gapple |
Comments
Comment #2
gappleNull Coalesce operator makes this a short fix for D9
Comment #3
gapplePlease give credit to @rang501 for providing the steps to reproduce in #3109201: null passed to hook_library_info_alter() after module installed causes fatal error
Comment #4
jaredsmith CreditAttribution: jaredsmith as a volunteer commentedLooks good to me... simple one-line patch.
Comment #5
cilefen CreditAttribution: cilefen commentedIs an empty file a valid YAML file?
Comment #6
poorva CreditAttribution: poorva as a volunteer and commentedIt looks good but is this valid? an empty file should not be the part of module.
Comment #7
anu.a_95 CreditAttribution: anu.a_95 at Zyxware Technologies commentedWas not able to reproduce the issue
[ Drupal 9.1.x-dev, PHP 7.3, MySQL 5.7.29 ]
I have followed the steps
I didn't get any errors as specified in the issue. When I checked the log messages, I found some warnings and for reference they are attached here.
Comment #8
gapple@anu.a_zyxware I've updated the repro steps - the fatal error also requires enabling a module that type hints its
hook_library_info_alter()implementation, such as locale.module. I tested using the demo_umami profile, which has locale enabled.The warning you posted is caused be the same issue - the same null is being passed to a `foreach`.
----
YAML can serialize / parse scalar values, so
false, an empty array (serialized to{}), and an empty file (parsed toNULL) are all valid as the sole contents of a file.The contents of *.libraries.yml is currently never validated against a schema, so as long as it parses as valid YAML any other issues with the contents are only exposed when code tries to use it.
Modules probably shouldn't have an empty libraries file, so there's an argument for throwing
InvalidDataTypeExceptionlike is done if the YAML cannot be parsed, but I don't see any harm in converting it to an empty array.Comment #9
joelpittetConsidering this function is supposed to return an array, and an empty libraries.yml shouldn't cause a fatal
Comment #10
larowlanThanks folks, can we get a simple test here that demonstrates that the issue is resolved.
Should be able to follow @gapple's instructions in #8 to mimic the conditions
Comment #11
larowlanComment #12
anu.a_95 CreditAttribution: anu.a_95 at Zyxware Technologies commentedApplied patch #3
Reproduced issue by installing a Drupal 9.1.x-dev instance with Umami profile for demonstration purpose (Since the fatal error also requires enabling a module that type hints its hook_library_info_alter() implementation, such as locale.module which is already enabled in umami -- as updated by the reporter in #8) and adding an empty libraries.yml file to a module and later installed and enabled csp module. The website won't load further.
The error log of apache shows an error as specified in the issue description.
After applying the patch, the website worked normally. Didn't find any other issue related to the application of the patch,
Comment #13
gappleThe test failed for me locally due to the PHP warning being raised, even before getting to the assertion.
Comment #15
thallesAfter #13 works fine to me!
But before #13:
Comment #18
catchCommitted/pushed to 9.1.x and cherry-picked back to 8.9.x, thanks!
Comment #20
rodrigoaguileraI think this is fixed now