Problem/Motivation
In #3416700: Handle invalid compressed ajax_page_state more gracefully we added protection against invalid library query strings, but AttachedAssets is a value object that could validate these internally for all cases.
Steps to reproduce
Proposed resolution
Add validation to ::setLibraries() and ::setAlreadyLoadedLibraries().
Remove the validation from AssetControllerBase::deliver().
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 3442980-5.patch | 872 bytes | dieterholvoet |
| #4 | 3442980-4.patch | 605 bytes | seanb |
Issue fork drupal-3442980
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
anybodyThanks @longwave! It would be great then to add information about which library broke, because currently this is hard to find out for site managers. They see "The libraries to include are encoded incorrectly." but have no clue which library is encoded incorrectly and what that means. Perhaps "encoded incorrectly" isn't the perfect wording also?
See https://www.drupal.org/project/asset_injector/issues/3444733#comment-155... for details and background.
Comment #3
catchI think we should open separate issue to add more information about the library to the error message so we can backport that to 10.2.x (and 10.3.x once 10.3.0 is out) - this issue will make things even stricter so will only be able to go into a minor release. I have to admit I didn't imagine that a module would specify a real library with two forward slashes in it, the validation is supposed to be against people trying to inject malicious information into the query parameter (e.g. disk filling attempts, things like that).
Comment #4
seanbCame here via #3348789: Compress ajax_page_state and #3416700: Handle invalid compressed ajax_page_state more gracefully. I also got the
Warning: Undefined array key 1 in Drupal\Core\Asset\LibraryDependencyResolver->doGetDependencies()error becausesystem_js_settings_alterended up returning an empty string. When this string is compressed and unpacked this ends up adding an empty library which can not be resolved.Regarding the solution, I think it would be best if
system_js_settings_altersimply doesn't add$settings['ajaxPageState']['libraries']when the list of libraries is empty.So instead of this:
We do this:
I attached a patch for anyone that needs it. I'm not sure if this is the correct issue to address it, I can create a separate issue if needed.
Comment #5
dieterholvoet commentedI re-rolled the patch for Drupal 11.1.x.