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.

Files: 
CommentFileSizeAuthor
#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 ]

Comments

Status:Active» Needs review

run tests.

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.

Status:Needs review» Reviewed & tested by the community

Er, meant to do this.

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

+1

+++ 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.

StatusFileSize
new3.41 KB
PASSED: [[SimpleTest]]: [MySQL] 58,378 pass(es).
[ View ]
new2.1 KB

Combining that and my docs additions as discussed in IRC.

Title:ParamaterConverter mangles raw valuesParameterConverter 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.

Status:Needs work» Needs review
StatusFileSize
new1.03 KB
new3.49 KB
PASSED: [[SimpleTest]]: [MySQL] 58,859 pass(es).
[ View ]

How about this?

Status:Needs review» Reviewed & tested by the community

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.

Issue summary:View changes

Updated issue summary.