Re: [Catalyst] RE: uri_for() corrupts query parameters hash for caller
On 26 Jun 2009, at 23:19, Byron Young wrote: Alrighty, here you go, patch + test are attached. There are based off the 5.71001 svn head because that's what I have currently. 5.8's uri_for() looks the same, so it should apply there as well, but let me know if you need another one from 5.8. http://dev.catalyst.perl.org/svnweb/Catalyst/revision/?rev=10736 Thanks very much for the patch, applied ok to 5.80 trunk. I rewrote your fix to just not mangle $_, which fixes the same issue with less code, and avoids the unsafe each.. Unfortunately, this just missed the Catalyst 5.80006 release, so you'll have to wait for the next one to see it in released code, sorry! Cheers t0m ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
RE: [Catalyst] RE: uri_for() corrupts query parameters hash for caller
Tomas Doran wrote on 2009-06-29: On 26 Jun 2009, at 23:19, Byron Young wrote: Alrighty, here you go, patch + test are attached. There are based off the 5.71001 svn head because that's what I have currently. 5.8's uri_for() looks the same, so it should apply there as well, but let me know if you need another one from 5.8. http://dev.catalyst.perl.org/svnweb/Catalyst/revision/?rev=10736 Thanks very much for the patch, applied ok to 5.80 trunk. I rewrote your fix to just not mangle $_, which fixes the same issue with less code, and avoids the unsafe each.. Unfortunately, this just missed the Catalyst 5.80006 release, so you'll have to wait for the next one to see it in released code, sorry! Cheers t0m Oh, nice, that's a much better solution. If you don't mind, though, can you explain what you mean about the 'unsafe each'? Thanks! Byron ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] RE: uri_for() corrupts query parameters hash for caller
On 30 Jun 2009, at 00:31, Byron Young wrote: If you don't mind, though, can you explain what you mean about the 'unsafe each'? If your application code half iterates through the params hash with each before calling uri_for, then the param copy would only copy the second half of the hash (as each has an internal iterator). Cheers t0m ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
RE: [Catalyst] RE: uri_for() corrupts query parameters hash for caller
Tomas Doran wrote on 2009-06-29: On 30 Jun 2009, at 00:31, Byron Young wrote: If you don't mind, though, can you explain what you mean about the 'unsafe each'? If your application code half iterates through the params hash with each before calling uri_for, then the param copy would only copy the second half of the hash (as each has an internal iterator). Cheers t0m Ah, makes sense. Learn something new every day! Thanks! Byron ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
[Catalyst] Re: uri_for() corrupts query parameters hash for caller
* Tomas Doran bobtf...@bobtfish.net [2009-06-30 02:35]: If your application code half iterates through the params hash with each before calling uri_for, then the param copy would only copy the second half of the hash (as each has an internal iterator). FWIW you can reset the iterator using `keys`, which is cheap to call in void or scalar context too. Regards, -- Aristotle Pagaltzis // http://plasmasturm.org/ ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] RE: uri_for() corrupts query parameters hash for caller
Byron Young wrote: I know people have been busy (I think there were some perl conferences lately?) and I think my issue slipped through the cracks. Just wanted to know what people thought about this and whether I should submit my patch or take a different approach. Sorry for dropping this on the floor. Yes, please submit a patch :) Cheers t0m ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
RE: [Catalyst] RE: uri_for() corrupts query parameters hash for caller
Tomas Doran wrote on 2009-06-26: Byron Young wrote: I know people have been busy (I think there were some perl conferences lately?) and I think my issue slipped through the cracks. Just wanted to know what people thought about this and whether I should submit my patch or take a different approach. Sorry for dropping this on the floor. Yes, please submit a patch :) Cheers t0m Alrighty, here you go, patch + test are attached. There are based off the 5.71001 svn head because that's what I have currently. 5.8's uri_for() looks the same, so it should apply there as well, but let me know if you need another one from 5.8. (I'm not sure how tracking contributors works, or if you do, but if so this was worked on by myself and Amir Sadoughi). Byron uri_for_patch.diff Description: uri_for_patch.diff ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
[Catalyst] RE: uri_for() corrupts query parameters hash for caller
Byron Young wrote on 2009-06-12:
Byron Young wrote on 2009-06-12:
Hey everybody,
I ran into an issue at $work where we keep passing the same
$query_params hashref to a number of uri_for() calls successively, but
if there are characters in the query params that need to be escaped
they get escaped each time, leading to sequences like
?filter=Not%25252BRun
after the same $query_params have been run through uri_for a few of
times (because the '%' keeps getting escaped). The query hash was
originally { filter = 'Not Run' }.
So, we patched uri_for() here at work to create a copy of $params
and work with that, and that fixes the issue. However, it seems
like such a simple fix that I feel like it must have been thought
of and discussed and shot down in the past, but I didn't find
anything in the list archives indicating that. Is there some
reason uri_for() does things that way?
If not I'll gladly supply patch + test.
Thanks,
Byron
I also noticed that the docs for uri_for used to warn of the
destructiveness but that warning has been removed in more recent
versions. I'd like to suggest that it be added back and made more
prominent if there really is a good reason for mangling the
caller's data. I can provide a doc patch in that case, too.
Byron
Hey,
I know people have been busy (I think there were some perl conferences lately?)
and I think my issue slipped through the cracks. Just wanted to know what
people thought about this and whether I should submit my patch or take a
different approach.
Thanks
Byron
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
[Catalyst] RE: uri_for() corrupts query parameters hash for caller
Byron Young wrote on 2009-06-12:
Hey everybody,
I ran into an issue at $work where we keep passing the same
$query_params hashref to a number of uri_for() calls successively,
but if there are characters in the query params that need to be
escaped they get escaped each time, leading to sequences like
?filter=Not%25252BRun
after the same $query_params have been run through uri_for a few of
times (because the '%' keeps getting escaped). The query hash was
originally { filter = 'Not Run' }.
So, we patched uri_for() here at work to create a copy of $params
and work with that, and that fixes the issue. However, it seems
like such a simple fix that I feel like it must have been thought
of and discussed and shot down in the past, but I didn't find
anything in the list archives indicating that. Is there some
reason uri_for() does things that way?
If not I'll gladly supply patch + test.
Thanks,
Byron
___ List:
Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-
bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-
archive.com/catalyst@lists.scsys.co.uk/ Dev site:
http://dev.catalyst.perl.org/
I also noticed that the docs for uri_for used to warn of the destructiveness
but that warning has been removed in more recent versions. I'd like to suggest
that it be added back and made more prominent if there really is a good reason
for mangling the caller's data. I can provide a doc patch in that case, too.
Byron
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
