Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
This is the problem code. There are two ways that the throw can never be reached.
if ($name instanceof SymfonyRoute) {
$route = $name;
}
elseif (NULL === $route = clone $this->provider->getRouteByName($name)) {
throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', $name));
}
1) This interface only allows returning of a route object. If it can't it already throws a RouteNotFoundException.
2) If it _did_ return NULL, it just triggers a fatal error because you can't clone NULL. https://3v4l.org/0YOG1
Steps to reproduce
You can return null from a route provider, like using a empty mock to trigger the fatal but the 3v4l link demonstrates the problem.
Proposed resolution
Remove the elseif logic.
Remaining tasks
n/a
User interface changes
n/a
API changes
n/a
Data model changes
n/a
Release notes snippet
n/a
Comment | File | Size | Author |
---|---|---|---|
#9 | 3184619-9.patch | 1.05 KB | neclimdul |
Comments
Comment #2
neclimdulThis should be an easy novice review.
Note, there are no tests because the code is unreachable so there isn't anything to test. Any tests that could trigger a problem would have to break interfaces.
Comment #4
neclimdulrandom failure
Comment #6
quietone CreditAttribution: quietone as a volunteer commentedWhat a nice little find in my triaging journey.
Starting tests on 9.3.x
Comment #7
quietone CreditAttribution: quietone as a volunteer commentedI read the IS and looked at https://3v4l.org/0YOG1 and read the code. It all looks good to me. And of course, no tests are failing.
So, RTBC from me.
Comment #8
quietone CreditAttribution: quietone as a volunteer commentedAnd it is going to rested on 9.2.x instead of 9.3.x. :-(
Comment #9
neclimdulHow nice of you to say!
Here's a patch with the use fix that should make CI happy again. Just going to leave it RTBC to save some noise on such a trivial fix.
Comment #12
catchCommitted/pushed to 9.4.x and cherry-picked to 9.3.x, thanks!