Problem/Motivation
After upgrading to version 8.x-2.4 the error message below started printing on any page with a carousel:
Warning: file_get_contents(/REDACTED/docroot/libraries/slick/package.json): failed to open stream: No such file or directory in Drupal\slick\SlickSkinManager->isBreaking() (line 432 of modules/contrib/slick/src/SlickSkinManager.php).
From what I can tell the module is trying to read the version of the JS library from the package.json file.
This is a problem because in order to prevent supply chain attacks we automatically remove any package.json files from JS libraries during our deployment process.
Steps to reproduce
1. Enable the module and install the JS library.
2. Remove the package.json file form the JS library folder.
3. Add a Slick carousel to a page.
4. Clear cache.
5. The error should be visible.
Proposed resolution
Add a fallback catch to not throw an error if the package.json file cannot be read. Only throw the rror if the file is read, but the version is not correct.
Remaining tasks
Decide on fix and implement.
User interface changes
None
API changes
None
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 3246544-6-error-package-json.patch | 1.24 KB | tanmayk |
Comments
Comment #2
gausarts commentedThank you.
The second by-product reported issue of the new awesomess: #3196529: Support Accessible Slick
The responsible method:
https://git.drupalcode.org/project/slick/-/blob/8.x-2.4/src/SlickSkinMan...
With the
@todoto remove it:So, the best solution is we should remove it.
We had enough babysitting codes around. This new questionable one should be gotten rid.
Instead we had provided relevant warnings at project home requirements section.
Patches are welcome. Thanks.
Comment #3
joelpittetYeah that is very unfortunate, npm is still reporting 1.8.1 as the latest in the ^1.0.0 releases. We are copying the library with npm's copy-files-from-to package but were excluding the top directory contents because none of it was executable code. Thanks for reporting this, the proposed solution is good, but won't help with this issue with the 1.8.1. Can we parse the slick.js file comment of the version number or is that silly?
Comment #4
joelpittetI like the removing the code entirely as @gausarts suggests in #2.
Comment #5
gausarts commented> Can we parse the slick.js file comment of the version number
Unfortunately, unlike package.json, versions at this file are inconsistent, so no:
https://github.com/kenwheeler/slick/blob/1.8.0/slick/slick.js
https://github.com/kenwheeler/slick/blob/v1.8.1/slick/slick.js
These two files have 1.8.0 as version, likely due to being hard-coded.. The package.json is strictly correct.
The problem is identified here as per deletion: https://github.com/kenwheeler/slick/pull/3688 and still present at v1.8.1.
Perhaps another reason why the recommended release has no v.1.8.1:
https://github.com/kenwheeler/slick/releases
The Latest marked as release is 1.8.0, not v.1.8.1.
It is starting to become a concern when #3196529: Support Accessible Slick was in, it inherits the troublesome v1.8.1 which cause confusing breakages.
>removing the code entirely
Only removing the
elsebranch:https://git.drupalcode.org/project/slick/-/blob/8.x-2.4/src/SlickSkinMan...
as OP reported.
The breaking method is still kept to minimize sudden breakages as mentioned above. This particular babysitter is still needed to minimize extra works due to issues between 1.8.0 vs 1.8.1 which severely break displays.
Unless we had time to re-update each optionset manually when changing from Accessible to regular Slick, and vice-versa.
Comment #6
tanmaykPatch against 2.x branch as per comments from @gausarts.
Comment #7
pcate commentedPatch #6 applied cleanly and fixed issue for me.
Comment #9
gausarts commentedCommitted. Thank you for contribution and patience.