Problem/Motivation
LibraryDiscoveryCollector::getLibraryDefinitions() calls
LibraryDiscoveryParser::buildByExtension() without handling
UnknownExtensionException. The extension name reaching this code path can be
client-supplied: AJAX requests send ajax_page_state[libraries], which lists
extension/library pairs previously loaded on the page.
When that list references an extension that does not exist — because a module
or theme was uninstalled while pages or the browser state were still cached,
or because the parameter was tampered with — the uncaught
UnknownExtensionException bubbles up as a fatal error / 500 response instead
of the library simply being treated as not loaded.
This was discovered while working on #2934063: Remove the workaround in \Drupal\Core\Installer\ExtensionListTrait::setPathname() (installer extension pathname
workaround removal) and split out because it is an independent fix with its
own failure mode and test surface.
Steps to reproduce
1. Open any page that performs AJAX requests (e.g. a view with AJAX pager).
2. Tamper the ajax_page_state[libraries] request value to include a
non-existing extension, e.g. fakeLibrary/fakeLibrary (or uninstall a
module that provided libraries on a cached page, then trigger AJAX from
that stale page).
3. The request fails with an uncaught
Drupal\Core\Extension\Exception\UnknownExtensionException instead of
ignoring the unknown library.
Proposed resolution
Catch UnknownExtensionException around LibraryDiscoveryParser::buildByExtension() in LibraryDiscoveryCollector::getLibraryDefinitions() and return an empty
library set for unknown extensions:
- getLibrariesByExtension('fakeLibrary') returns [].
- getLibraryByName('fakeLibrary', 'fakeLibrary') returns FALSE.
Scope boundaries:
- Only `UnknownExtensionException` is caught; other parser failures
(`InvalidLibrariesExtendSpecificationException`,
`InvalidLibrariesOverrideSpecificationException`) keep throwing — those
indicate broken definitions of *known* extensions and must stay visible.
- No change to asset library validation semantics for known extensions.
- No installer/pathname API changes (that is #2934063: Remove the workaround in \Drupal\Core\Installer\ExtensionListTrait::setPathname()).
Remaining tasks
- review
- decide whether functional/Ajax-level regression coverage is wanted on top of the unit coverage.
User interface changes
no
Introduced terminology
no
API changes
None. LibraryDiscoveryCollector::getLibrariesByExtension() now returns an empty array for unknown extensions instead of letting UnknownExtensionException escape; getLibraryByName() consequently returns FALSE. Both are relaxations of failure behavior, not signature changes.
Data model changes
no
Release notes snippet
no
Issue fork drupal-3601471
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
andypostRelated CR https://www.drupal.org/node/2941753
Comment #4
andypostComment #5
alexpottI think this is a good idea. Tampering with input shouldn't lead to this exception - so we log this somewhere though or does this change it to become a silent error?
Comment #6
andypostYes, it should be logged with warning as
asset.js.optimizerto default logger, as it could be both a bug of frontend and some vectorComment #7
andypostadded CR and logger