Fatal error due to saved additional form IDs being strings and not arrays

This patch looks good and it does the job. Marking as RTBC.

I applied the patch using composer like this:

  - Installing drupal/readonlymode (dev-1.x 58e8b27): Cloning 58e8b2774c from cache
  - Applying patches for drupal/readonlymode
    https://www.drupal.org/files/issues/2020-02-21/readonlymode-string_not_array-3115119-2.patch (Got fatal error when running this module)

But when I open page with a webform block as an anonymous user, I get this fatal error:

The website encountered an unexpected error. Please try again later.
TypeError: Argument 2 passed to _readonlymode_form_list_check() must be of the type string, array given, called in /var/www/html/docroot/modules/contrib/readonlymode/readonlymode.module on line 228 in _readonlymode_form_list_check() (line 249 of modules/contrib/readonlymode/readonlymode.module). 

So it seems that this patch is actually causing the problem. Can try to replicate my use case, please?

I am unable to reproduce this error after installing the module. Can someone provide the steps to reproduce?

I've encountered this, as well. It occurs on pages with forms when installing the module without having altered its settings via the UI or settings.php.

I noticed that in config/install/readonlymode.settings.yml the settings of forms.additional are arrays, but when saving the module's settings they are strings.

It can be fixed by overriding in settings.php with the following (if no change is desired and the settings should be empty):

$config['readonlymode.settings']['forms']['additional']['view'] = '';
$config['readonlymode.settings']['forms']['additional']['edit'] = '';

I also ran into this while updating a site.
must be of the type array, string given

I commented the lines calling the _readonlymode_form_list_check() function to get through the drush updb process.

The second argument get's it's value from $settings->get('forms.additional.view') so apparently that varies in type(#8)
.
Looking at the next line it uses preg_split which is a string function to process the argument. The functions comment section also describes the argument as 'A string of form id's separated by newlines.'

The last change to this line was in a 'Coder review and drupal-check changes.' commit which added the argument type. I think that if we just remove it we're back to a working state.
When an array is passed in anyway it generates a PHP Warning: preg_split() expects parameter 2 to be string, array given (not fatal, but still not nice)

The patch added here adds a check to avoid that warning.

I've just applied #9 patch on 1.1 and it solves that bug. Thanks @Helmo !

Patch #9 worked for me. Thanks.

I had the same error the Patch #9 fixed the fatal error for me.
but i had other errors after i applied it
When user don't have the "Access all forms while in Read Only Mode" permission and tried to access to content page (admin/content)

 Notice: Undefined index: show_view_elements in Drupal\views\Plugin\views\display\DisplayPluginBase->elementPreRender() (line 2261 of core/modules/views/src/Plugin/views/display/DisplayPluginBase.php).
Notice: Trying to access array offset on value of type null in Drupal\views\Plugin\views\display\DisplayPluginBase->elementPreRender() (line 2261 of core/modules/views/src/Plugin/views/display/DisplayPluginBase.php).

I can second what @berramou said. Since this is a less ugly error I'll use the patch, but it's not ready.

I ran into this error as well. Looking over the previous patches, they only address the symptom but not the root cause: that the configuration being saved isn't arrays of form IDs (as one would expect) but strings with new lines in them. I've opened a merge request that splits the fields into arrays before saving them to config, and also added an update hook to do the same thing for existing config.

I tested MR 4 from @Ambient.Impact and found that I still see the same error. Here's what I did:

• Applied patch from MR (https://git.drupalcode.org/project/readonlymode/-/merge_requests/4.patch).
• Ran database updates (MR includes a hook_update_N).
• Logged in as user with "Access all forms while in Read Only Mode" permission and enabled Read Only Mode at /admin/config/development/maintenance.
• Logged in as user without "Access all forms while in Read Only Mode" permission and went to /admin/content.
• I see the message I configured to appear when Read Only Mode is enabled, and I also see these errors:
  

  Warning: Undefined array key "show_view_elements" in Drupal\views\Plugin\views\display\DisplayPluginBase->elementPreRender() (line 2261 of core/modules/views/src/Plugin/views/display/DisplayPluginBase.php).
  Drupal\views\Plugin\views\display\DisplayPluginBase->elementPreRender(Array)
  call_user_func_array(Array, Array) (Line: 101)
  Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 772)
  Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array) (Line: 363)
  Drupal\Core\Render\Renderer->doRender(Array) (Line: 435)
  Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 201)
  Drupal\Core\Render\Renderer->render(Array, ) (Line: 241)
  Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 564)
  Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 242)
  Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 132)
  Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
  Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
  call_user_func(Array, Object, 'kernel.view', Object) (Line: 142)
  Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.view') (Line: 174)
  Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
  Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
  Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
  Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
  Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
  Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
  Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
  Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
  Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
  Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
  Warning: Trying to access array offset on value of type null in Drupal\views\Plugin\views\display\DisplayPluginBase->elementPreRender() (line 2261 of core/modules/views/src/Plugin/views/display/DisplayPluginBase.php).
  Drupal\views\Plugin\views\display\DisplayPluginBase->elementPreRender(Array)
  call_user_func_array(Array, Array) (Line: 101)
  Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 772)
  Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array) (Line: 363)
  Drupal\Core\Render\Renderer->doRender(Array) (Line: 435)
  Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 201)
  Drupal\Core\Render\Renderer->render(Array, ) (Line: 241)
  Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 564)
  Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 242)
  Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 132)
  Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
  Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
  call_user_func(Array, Object, 'kernel.view', Object) (Line: 142)
  Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.view') (Line: 174)
  Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
  Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
  Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
  Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
  Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
  Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
  Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
  Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
  Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
  Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
  

I'm sorry if this is frowned upon but I added a small commit to MR 4 to fix the error I described in my last comment. With that added I don't see any other errors.

It seems the way we are unsetting form elements may be overly aggressive and is the cause of Warning: Undefined array key "show_view_elements". Wouldn't setting [#access] = FALSE be safer? To be clear, this isn't a criticism of MR 4. The code I'm talking about already exists. See the foreach loop here at line 151 here: https://git.drupalcode.org/project/readonlymode/-/merge_requests/4/diffs...

I'm sorry if this is frowned upon but I added a small commit to MR 4 to fix the error I described in my last comment. With that added I don't see any other errors.

Not at all, issue forks are intended for this kind of collaboration so additional work is appreciated. It would be preferable to discuss additional changes beforehand however. In this case, I'm curious if you can replicate the the undefined array key warning using the dev branch (i.e. not this issue fork), and if so, then you should open a separate issue and create a new issue fork in that issue rather than in this one. On the other hand, if my merge request does indeed cause this warning, then it would be appropriate here.

It seems the way we are unsetting form elements may be overly aggressive and is the cause of Warning: Undefined array key "show_view_elements". Wouldn't setting [#access] = FALSE be safer?

Agreed that using #access would likely be a lot safer and the better approach. Feel free to open a new issue and merge request with that change. 😉

Was this fixed in #3224538: Config schema errors? Or is it partially fixed?
The merge request here is all about arrays, while the other issue further stringified things.

And more: #3354694: Argument 2 passed to _readonlymode_form_list_check() must be of the type array, string given

Uploading the patch generated from https://git.drupalcode.org/project/readonlymode/-/merge_requests/4/diffs... based on comment 19.

Don't be mad y'all, I know there are plenty of conversations around uploaded patches vs. using MR generated patches locally. For me, I'm contributing this patch to another project that doesn't have local patches, so I want a permanent link. So far as I can tell this is not yet supported in GitLab.

Patch in #23 did not work for me on D10.

In admin/config/development/maintenance, I got:
TypeError: implode(): Argument #2 ($array) must be of type ?array, string given in implode() (line 102 of modules/contrib/readonlymode/readonlymode.module).

I could get a no error or warning install using

1. Using https://www.drupal.org/files/issues/2023-04-17/argument-passed-as-string... fixed TypeError: Argument 2 passed to _readonlymode_form_list_check() must be of the type string, array given. Credit to OPTASY
2. Using https://git.drupalcode.org/project/readonlymode/-/commit/2ef5cf6e49dbcca... fixed Warning: Undefined array key "show_view_elements". Credit to Ben Teegarden
3. There was also a "Warning: Undefined array key "actions"" in some administrative pages that I fixed appending the 'actions' key just like in item 2.

Just fixing filename from #24.

Same patch as #24 but remove only 'show_view_elements' element from form.

We are using patch #23 successfully in production sites.

@Imlima If you were getting the TypeError: implode(): Argument #2 ($array) must be of type ?array, string given in implode() and TypeError: Argument 2 passed to _readonlymode_form_list_check() must be of the type string, array given errors after applying patch #23 then you hadn't run drush updatedb to run readonlymode_update_8002() which converts the 'forms.additional.edit' and 'forms.additional.view' config items into arrays? Or had they errored and failed somehow?

There are now two ways of fixing the issue in patches, up to and including patch #23 and in the merge request it was all about making the two config vars arrays as the code was expecting, patch #25 and #27 are simpler by just changing what the code to expecting the strings as they are in the current config.

Personally I favour the first solution to make the config arrays as that is what the code uses in _readonlymode_form_list_check(). If the config is saved as an array then it is only converted from string to array and back again once in the admin form. When it is being used during normal site operation the config is an array.
By keeping it as a string in the config every time that _readonlymode_form_list_check() is called the string has to be split into an array at that point. Not a massive overhead but it seems that it is better to store the config how it is intended to be used, not just as a consequence of how it is edited in the admin form?

As mentioned in comment #21 above (#3115119-21: Fatal error due to saved additional form IDs being strings and not arrays),
it seems this patch may no longer be needed as the 1.3 release fixed the config schema by correctly specifying these values as strings rather than converting the storage to arrays: #3224538: Config schema errors.

It looks like upgrading to 1.3 , re-configuring the module, and re-exporting the config would fix things.

edit: after looking at this more and discussing with affected users, I think we need to follow through with the idea in the previous comment (#28) and get the module updated so these values are treated properly as arrays at all times + db update if needed.

it's a duplicate of #3354694: Argument 2 passed to _readonlymode_form_list_check() must be of the type array, string given