- Issue created by @aduthois
Also, I don't know what this is about. Can you add some more detail there please?
Route '/user/password' no '_format' requirement
explode('|', $route->getRequirement('_format')) with null when $route_name is 'user.pass.http' :
public function onRoutingAlterAddFormats(RouteBuildEvent $event) {
$route_names = [
'user.login_status.http',
'user.login.http',
'user.logout.http',
'user.pass.http',
];
$routes = $event->getRouteCollection();
foreach ($route_names as $route_name) {
if ($route = $routes->get($route_name)) {
$formats = explode('|', $route->getRequirement('_format'));
$formats = array_unique(array_merge($formats, $this->serializerFormats));
$route->setRequirement('_format', implode('|', $formats));
}
}
}- Status changed to Postponed: needs info
almost 2 years ago 11:06am 12 May 2023 This issue needs some editing to be understandable to contributors.
- 🇫🇷France Manuel.loth
Hello,
I have the same issue.
a call to drupal_flush_all_caches() results in the following warning :Deprecated function: explode(): Passing null to parameter #2 ($string) of type string is deprecated in Drupal\serialization\EventSubscriber\UserRouteAlterSubscriber->onRoutingAlterAddFormats() (line 55 of core/modules/serialization/src/EventSubscriber/UserRouteAlterSubscriber.php). Drupal\serialization\EventSubscriber\UserRouteAlterSubscriber->onRoutingAlterAddFormats(Object, 'routing.route_alter', Object) call_user_func(Array, Object, 'routing.route_alter', Object) (Line: 111) Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'routing.route_alter') (Line: 189) Drupal\Core\Routing\RouteBuilder->rebuild() (Line: 83) Drupal\Core\ProxyClass\Routing\RouteBuilder->rebuild() (Line: 518) drupal_flush_all_caches() (Line: 316) Drupal\bofip_za_documents\Form\ActuAlaUneForm->submitForm(Array, Object) call_user_func_array(Array, Array) (Line: 114) Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 52) Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 597) Drupal\Core\Form\FormBuilder->processForm('actu_ala_une_form', Array, Object) (Line: 325) Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73) Drupal\Core\Controller\FormController->getContentResult(Object, Object) call_user_func_array(Array, Array) (Line: 123) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 580) Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 163) Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 74) 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: 51) Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 686) Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
So it seems to be caused by the absence of parameter '_format' in the 'user.pass' route in web/core/modules/user/user.routing.yml, as can be seen in the code posted above that I quote again :
\Drupal\serialization\EventSubscriber\UserRouteAlterSubscriber::onRoutingAlterAddFormats, line 55 :$formats = explode('|', $route->getRequirement('_format'));
Apparently, _format is now a required parameter for all routes, but it is not present in user.pass.
So the proposed resolution should be to fix the route by adding this parameter.
I cannot reproduce this. I installed Drupal 10.0.x on PHP 8.2 with full error reporting.
Are the affected sites running patches on Drupal Core?
onRoutingAlterAddFormats is only evaluating these routes:
$route_names = [ 'user.login_status.http', 'user.login.http', 'user.logout.http', 'user.pass.http', ];
user.pass
is not one of them.user.pass.http
is on of them, but it has the_format
configured.- Status changed to Active
almost 2 years ago 2:39pm 16 May 2023 The problem came from a contrib ldap module which overrides the 'Requirements' parameter of user.pass.http.
It would still be useful to check the 'explode()' method for nulls. THANKSBut those four routes require it. Why would it be good to silently work around the missing value?
- Status changed to Closed: works as designed
almost 2 years ago 5:49am 17 May 2023 - First commit to issue fork.
- Merge request !8841Issue #3359649: Deprecated function: explode(): Passing null to parameter #2... → (Open) created by arnested
- Status changed to Needs review
8 months ago 11:50am 19 July 2024 - 🇩🇰Denmark arnested
I'm reopening this and adding a suggested fix.
I'm running into this issue as well.
My use case is that I want to deny access to
/user/password
(we use an external login provider).In a
RouteSubscriber::alterRoutes()
we do:if ($route = $collection->get('user.pass.http')) { $route->setRequirements([]); $route->setRequirement('_access', 'FALSE'); }
We clear all existing requirements and then set access to FALSE because we want to be sure no other requirements interfere.
But then there is no format on the route (and none should be needed).
- Status changed to Needs work
8 months ago 12:48pm 19 July 2024 The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- Status changed to Needs review
8 months ago 1:30pm 19 July 2024 - Status changed to Needs work
8 months ago 1:53pm 19 July 2024 - 🇺🇸United States smustgrave
believe the conversation around 8-10 still applies. Seems like this could be an issue with some contrib module.
- Merge request !8848Issue #3359649 by arnested, aduthois, cilefen, Manuel.loth, smustgrave:... → (Open) created by arnested
- Status changed to Needs review
8 months ago 5:10pm 19 July 2024 - 🇩🇰Denmark arnested
OK, I got a test that triggers the deprecation notice.
The deprecation notice occurs when some other module/code has altered away the
_format
on, e.g., theuser.pass.http
route.But now I'm unsure what the serialization module should actually do when coming across such a route. Should it still alter the route and add
json|xml
as_format
, or should it refrain from adding an altered format back in?My first thought was to not add it, but on a second thought, I think it should. If you really want the format gone, you should probably make sure your event subscriber runs after the serialization module's event subscriber.
Issue summary updated.
- 🇩🇰Denmark arnested
arnested → changed the visibility of the branch 3359649-deprecated-function-explode to hidden.
- 🇮🇳India arunkumark Coimbatore
I was able to replicate the issue from my side. I followed the below steps to reproduce,
1. Installed custom module having the logic to alter the Route.
2. Altering RouteSubscriberBase as below.
3. Clear the cache, Access the route user/password
4. Getting below errorDeprecated function: explode(): Passing null to parameter #2 ($string) of type string is deprecated in Drupal\serialization\EventSubscriber\UserRouteAlterSubscriber->onRoutingAlterAddFormats() (line 55 of /var/www/html/docroot/core/modules/serialization/src/EventSubscriber/UserRouteAlterSubscriber.php) #0 /var/www/html/docroot/core/includes/bootstrap.inc(164): _drupal_error_handler_real(8192, 'explode(): Pass...', '/var/www/html/d...', 55) #1 [internal function]: _drupal_error_handler(8192, 'explode(): Pass...', '/var/www/html/d...', 55) #2 /var/www/html/docroot/core/modules/serialization/src/EventSubscriber/UserRouteAlterSubscriber.php(55): explode('|', NULL)
After checkout to the Fix MR.
Am good on moving to RTBC
LGTM
- Status changed to RTBC
8 months ago 11:38am 26 July 2024 - 🇳🇿New Zealand quietone
I read the issue summary, the comments and the MR (not a code review). There are few things still to do, not in any particular order.
1. The issue title is an error message. The issue title should be a a description of what is being fixed here. See https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... →
2. The proposed resolution → section should be be a description of the change so that a reviewer knows what should be implemented.
3. I see that this was closed once as working as designed. This was re-opened and information about that was asked for in #17 but I don't see an answer. Have I missed something?
4. I also don't see any evidence of a code review. - 🇮🇳India arunkumark Coimbatore
Updated issue summary and title as recommended in #23
- Status changed to Needs work
7 months ago 11:29am 29 July 2024 - Status changed to Needs review
7 months ago 11:40am 29 July 2024 - Status changed to RTBC
7 months ago 7:01pm 29 July 2024 - 🇳🇿New Zealand quietone
This is better shape now, thanks.
However, I still don't see that someone has actually looked and the code and reviewed it.
- Status changed to Needs work
7 months ago 1:24pm 9 August 2024 - Status changed to Needs review
7 months ago 3:39pm 9 August 2024 - Status changed to RTBC
7 months ago 3:32pm 14 August 2024 - 🇺🇸United States smustgrave
https://git.drupalcode.org/issue/drupal-3359649/-/jobs/2397062 still shows test coverage.
See code change has been addressed and 1 question answered.
- Status changed to Needs work
6 months ago 4:09pm 11 September 2024 - First commit to issue fork.
- Assigned to shalini_jha
- Issue was unassigned.
- Status changed to Needs review
6 months ago 11:54am 19 September 2024 - 🇮🇳India shalini_jha
"I have updated the code based on PR feedback and fixed the pipeline for the failing test after modifying UserRouteAlterSubscriber.php. However, I'm still facing pipeline failures, which may be due to the MR needing a rebase. I attempted to rebase, but it shows that everything is up to date. Moving forward with the NR, please guide if I'm missing something in the rebase process."
- 🇺🇸United States smustgrave
Nightwatch keeps failing, needs to be investigated
- 🇩🇰Denmark arnested
I couldn't make any sense out of the nightwatch failure.
I did a rebase on 11.x and now the tests are all green.
- 🇺🇸United States smustgrave
Ran the test-only feature
There was 1 failure: 1) Drupal\Tests\serialization\Kernel\UserRouteAlterTest::testUserAlteredRouteWithoutFormat user.pass.http route does not have "_format" requirement Failed asserting that an array does not have the key '_format'. /builds/issue/drupal-3359649/core/modules/serialization/tests/src/Kernel/UserRouteAlterTest.php:73 -- 2 tests triggered 1 PHP deprecation: 1) /builds/issue/drupal-3359649/core/modules/serialization/src/EventSubscriber/UserRouteAlterSubscriber.php:55 explode(): Passing null to parameter #2 ($string) of type string is deprecated Triggered by: * Drupal\Tests\serialization\Kernel\UserRouteAlterTest::testUserAlteredRoute /builds/issue/drupal-3359649/core/modules/serialization/tests/src/Kernel/UserRouteAlterTest.php:51 * Drupal\Tests\serialization\Kernel\UserRouteAlterTest::testUserAlteredRouteWithoutFormat /builds/issue/drupal-3359649/core/modules/serialization/tests/src/Kernel/UserRouteAlterTest.php:64 FAILURES!
Which showed covered, and both tests failed nice!
Reviewing issue summary and that appears complete. Has steps to reproduce and solutions matches what's in MR.
Reviewing code changes in MR and addition of the check for _format makes sense, rest appears around the test coverage.
LGTM
-
alexpott →
committed 059150ef on 10.3.x
Issue #3359649 by arnested, shalini_jha, alexpott, arunkumark, cilefen,...
-
alexpott →
committed 059150ef on 10.3.x
-
alexpott →
committed 19853cff on 10.4.x
Issue #3359649 by arnested, shalini_jha, alexpott, arunkumark, cilefen,...
-
alexpott →
committed 19853cff on 10.4.x
-
alexpott →
committed 3b606077 on 11.0.x
Issue #3359649 by arnested, shalini_jha, alexpott, arunkumark, cilefen,...
-
alexpott →
committed 3b606077 on 11.0.x
-
alexpott →
committed a6a40068 on 11.x
Issue #3359649 by arnested, shalini_jha, alexpott, arunkumark, cilefen,...
-
alexpott →
committed a6a40068 on 11.x
- 🇬🇧United Kingdom alexpott 🇪🇺🌍
Committed and pushed to 11.x,11.0.x, 10.4.x and 10.3.x. Thanks!
Automatically closed - issue fixed for 2 weeks with no activity.