Limit what can be called by a callback in form arrays

It would be nice if this could make use of #2982950: [meta] Standardize the approach for capturing and invoking callables across various subsystems.

This one is probably critical.

Unless we make an exception, the new deprecation message would need to be 9.1.x-only. It might be OK to backport any new interfaces or similar without an exception message though.

I have some time to help with this - do maintainers want to deal with #4 first or should I just work at understanding and copying the approach in #2966327: Limit what can be called by a callback in render arrays to reduce the risk of RCE?

To invite direction from more experienced core contributors and promote collaboration, I'm going to comment on inconsistencies that I find as I work. Starting with #process simply because it was first.

In testing this refactoring, I encountered function user_form_process_password_confirm($element) {} which is declared as a #process callback in user_element_info_alter(). Now as documented in FormElement a #process callback should expect to receive $element, $form_state, $form. Based on the prior work I expect that we would deprecate user_form_process_password_confirm and I'm moving this logic to a static method on class PasswordConfirm.

In prior work the internals of the deprecated function is replaced with a call to the replacement static method, but if the static method is constructed to conform to the expected signature, we now have too few parameters. But changing the function signature of function user_form_process_password_confirm($element) {} would be a breaking change - so pending other guidance I'm refactoring function user_form_process_password_confirm($element) {} as follows:

function user_form_process_password_confirm($element) {
  // Refactored method expects the full #process parameters.
  $unused_form_state = new FormState();
  $unused_form = [];
  return PasswordConfirm::dynamicPasswordConfirm($element, $unused_form_state, $unused_form);
}

and conforming PasswordConfirm::dynamicPasswordConfirm to the signature expected by FormElement and FormBuilder.

First draft of #process pushed up

First draft of #after_build pushed up

First draft of #value_callback pushed

I pulled down the drupalci box and am re-running the last test run locally to try to get to the bottom of those last 3 failures before moving on.

Still updating for #validate

Revert vandalism https://www.drupal.org/project/site_moderators/issues/3276540

Untagging for release manager review - we can copy what we did in the render system issue.

While working on related I found that callbacks on PHP 8.0+ could be callables so we need to improve checking anyway for future

I'm able to get back to this issue after a good while dealing with other things. I'm going to look over the work I did before against 9.2 and update it against 10.1 in light of #3354584: Deprecate TrustedCallbackInterface in favour of TrustedCallback attribute

Refactoring against the 11.x branch

Thanks for changing the status @andypost. I’ll switch it when I finish this refactor

Tests finally passing for #process on 11.x

#after_build passed tests.

I’m looking at this section of code in FormBuilder

      // @todo Once all elements are converted to plugins in
      //   https://www.drupal.org/node/2311393, rely on
      //   $element['#value_callback'] directly.
      $value_callable = !empty($element['#value_callback']) ? $element['#value_callback'] : 'form_type_' . $element['#type'] . '_value';
      if (!is_callable($value_callable)) {
        $value_callable = '\Drupal\Core\Render\Element\FormElement::valueCallback';
      }

The issue #2311393: Remove hook_element_info() and all references to it is long closed with associated change record published 9 years ago. We have no form_type_TYPE_value functions in core now. It seems that this can be simplified and the @todo removed.

@FatherShawn re #46 is that #3221798: Remove stale processing and reference to form_type_TYPE_value, I think I came to the same conclusion a few years back.

I fixed #3221798: Remove stale processing and reference to form_type_TYPE_value as I was working on #value_callback here before I saw your comment @larowan. Thanks for connecting them! Can we simply close that issue when we close this one?

I would try to keep the scope as small as possible to ensure faster progress

Removing scope creep

Thanks for the review and the notes! If they prompt any questions I'll post on the MR - love that we can do that now!

RenderCallbackInterface and ElementInterface needs special approach but controllers are fixed in #3354584-35: Deprecate TrustedCallbackInterface in favour of TrustedCallback attribute