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.It is possible to implement hook_scald_atom_providers_opt_alter() in order to bypass the initial add form in scald. When doing so, the following steps in most of the scald providers will blindly try to access something in $form_state['values']... without even checking if $form_state['values']... is empty or not. This causes several notices to be displayed on the screen.
This happens in all the scald providers that we have found so far. Here is a list ...
- scald_audio (core)
- scald_flash (core)
- scald_image (core)
- scald_video (core)
- scald_file (separate repo)
- scald_text (separate repo)
- scald_views (separate repo)
We have written patches for the scald version that is on our servers. We are going to port them on the dev version so that anyone can benefit from it.
We will also submit the appropriate patches on scald_file, scald_text and scald_views repositories separately.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | scald-protect-bypassing-steps-2612714-5.patch | 596 bytes | gifad |
Comments
Comment #2
asiby CreditAttribution: asiby commentedComment #3
asiby CreditAttribution: asiby as a volunteer and commentedUpdate: We were using sclad 1.4 were the issue was first discovered. But after verification, version 1.6 is also affected by the same issue.
Comment #4
gifad CreditAttribution: gifad commentedThis 'starting point' feature was intended to allow new providers to not implement those two hooks (
hook_scald_add_atom_count()andhook_scald_add_form_fill()), where there would not be anything useful to do.Now if you alter existing providers, please do not patch their functions, just remove them !
Or better, instruct scald core not to call them (we are in a ctools modal multiform wizard, and that's not easy)
The attached patch has just two effective lines, the rest is indentation...
Comment #5
gifad CreditAttribution: gifad commentedA perhaps more elegant patch...
NB: tested with scald_audio, scald_file, scald_image, scald_video, scald_iframe, and, of course, scald_gallery.
Comment #6
nagy.balint CreditAttribution: nagy.balint commentedIt would be nice to still keep the functionality that the provider defines how many entities to create with the count hook even if the step is skipped.
Comment #7
nagy.balint CreditAttribution: nagy.balint commentedNeeds work since #6.
Did we miss something when we worked on this last time? #2580337: PHP notices while editing atom in CTools modal box
Comment #8
gifad CreditAttribution: gifad commentedabout #6 : This issue is about a third party module disabling the add step, so implicitly it disables the multi upload feature...
This patch has nothing to do with that.
What this issue is pointing out is that the 'starting_step' feature was intended to be used by a provider which does not want to supply the first step functions - and scald core was blindly calling them, just because they exist...
Comment #9
nagy.balint CreditAttribution: nagy.balint commentedIn the "scald_add_atom_count" the provider can define how many entities it wants created.
This can be because there is plupload, but it can be for any other reason as well.
It can be that a provider has a custom function to define how many entities it wants, when the provider has no upload mechanism, so its related to some other thing.
It can also be that a provider hard wires that it always wants 2 entities lets say to be created.
So that hook has to be called whether the first step was skipped or not.
In your patches that hook would not be called if the first step is skipped, therefore we loose the above functionality.
In my opinion we should not loose that functionality, and therefore I cant approve the above patches.
Comment #10
gifad CreditAttribution: gifad commentedAgain, my patch is in the context of
if (empty($form_state['scald']['atoms'])), it has nothing to do with loosing any functionality, it just fixes the bug of referencing unset variables; anyway, next week scald will migrate to Media, so I give up.Comment #11
nagy.balint CreditAttribution: nagy.balint commentedI suppose you mean Media Entity, and Drupal 8, but that does not mean that scald should not be maintained anymore for drupal 7...
As for the other part. If that form state is empty, it means that it is a new atom creation with the first step skipped. Yet that hook should be able to define how many entities to create, and therefore i still keep that my problem is valid.
Comment #12
asiby CreditAttribution: asiby as a volunteer and commented