[Catalyst] Dumper bugs (or why each() is dangerous full stop)

[Matt S Trout]

Bernhard Graf wrote:
> Hello,
> 
> today I encountered a really annoying behaviour (= potential bug) in 
> Catalyst:
> 
> I was writing a plugin that occasionally modifies 
> $c->request->body_parameters and $c->request->query_parameters .
> 
> This is done by defining a method prepare_body_parameters() in the 
> plugin, expecting prepare_parameters() to run later to merge body and 
> query parameters into $c->request->parameters().
> 
> But prepare_parameters() fails, because it uses each(). each() uses an 
> internal iterator. Previous modifications to the query & body params
> might leave iterators of these hashes at a random position, probably
> at the end - as a consequence the while ( ... each ...) loops aren't 
> executed at all.

This isn't normally a problem with correctly-written code.

Unfortunately, the XSUB version of Data::Dumper is not correctly 
written; if the sortkeys option is set, which it generally is for 
Catalyst debug output, this can leave the each() iterator in an 
incorrect position thus causing the exact behaviour you described.

And of course anybody else's buggy code can cause the same problem, 
including your own.

DBIx::Class has had a cleanup job where I removed the (sum total two) 
instances of each that had crept in; the Catalyst core should probably 
get the same.

My general rule of thumb is "unless you own the hash and never pass it 
anywhere else, don't use each() unless you reset the iteration position 
with keys in void context first."