Not sure if this is php version specific but through some tricks of references raw_variables get the converted values changed. The attached patch explicitly copies the array into a new array. This breaks references, try running php -r '$a = 1; $b = array(&$a); foreach ($b as $v) $v++; var_dump($a);' and see that despite $b[0] is a reference to $a, when foreaching over it, the reference is not preserved. Tests to confirm bug and fix.

Thanks to @dawehner for the test and figuring out where the bug was and @chx and @eclipsegc for talking it through and helping us figure out what was causing it.

#9 2103145-8.patch3.49 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,859 pass(es). View
#9 interdiff-2103145-8.txt1.03 KBdamiankloip
#6 interdiff.txt2.1 KBtim.plunkett
#6 MANGLES-2103145-6.patch3.41 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 58,378 pass(es). View
phpconverter_rawvalue_reference.patch3.06 KBneclimdul
PASSED: [[SimpleTest]]: [MySQL] 58,690 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more


neclimdul’s picture

Status: Active » Needs review

run tests.

tim.plunkett’s picture

Priority: Normal » Major
Issue tags: +WSCCI

I hit this while working on #2091691: Convert test non-form page callbacks to routes and controllers. Very confusing, glad it wasn't me.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Er, meant to do this.

Issue #2103145 by neclimdul, chx, dawehner: ParamaterConverter mangles raw values.

dawehner’s picture


chx’s picture

+++ b/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.phpundefined
@@ -156,7 +156,12 @@ public function enhance(array $defaults, Request $request) {
+    // Explicitly copy values to break references.

// foreach copies the values from the array it iterates even if they are references so use it to break references.

tim.plunkett’s picture

3.41 KB
PASSED: [[SimpleTest]]: [MySQL] 58,378 pass(es). View
2.1 KB

Combining that and my docs additions as discussed in IRC.

catch’s picture

Title: ParamaterConverter mangles raw values » ParameterConverter mangles raw values
Status: Reviewed & tested by the community » Needs work

From the comment I don't understand what's going to happen if we don't break those references - it'd be good to expand that a bit with an extra sentence or two to explain that the raw values end up getting replaced with the converted values if we don't do this. Bug took a long time to track down so worth adding as verbose docs as possible.

damiankloip’s picture

Status: Needs work » Needs review
1.03 KB
3.49 KB
PASSED: [[SimpleTest]]: [MySQL] 58,859 pass(es). View

How about this?

neclimdul’s picture

Status: Needs review » Reviewed & tested by the community
catch’s picture

Status: Reviewed & tested by the community » Fixed

OK that's great. And the test means it shouldn't get broken again anyway.

Committed/pushed to 8.x, thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.