FAQ
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

Search Discussions

  • Byron Young at Jun 12, 2009 at 9:48 pm

    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
  • Byron Young at Jun 25, 2009 at 7:51 pm

    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
  • Tomas Doran at Jun 26, 2009 at 3:02 pm

    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
  • Byron Young at Jun 26, 2009 at 10:19 pm

    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

    -------------- next part --------------
    A non-text attachment was scrubbed...
    Name: uri_for_patch.diff
    Type: application/octet-stream
    Size: 2224 bytes
    Desc: uri_for_patch.diff
    Url : http://lists.scsys.co.uk/pipermail/catalyst/attachments/20090626/20c23735/uri_for_patch.obj
  • Tomas Doran at Jun 29, 2009 at 11:23 pm

    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/?rev736

    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
  • Byron Young at Jun 29, 2009 at 11:31 pm

    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/?rev736

    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
  • Tomas Doran at Jun 30, 2009 at 12:32 am

    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
  • Byron Young at Jun 30, 2009 at 12:41 am

    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
  • Aristotle Pagaltzis at Jun 30, 2009 at 5:29 am

    * Tomas Doran [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/>

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupcatalyst @
categoriescatalyst, perl
postedJun 12, '09 at 9:43p
activeJun 30, '09 at 5:29a
posts10
users3
websitecatalystframework.org
irc#catalyst

People

Translate

site design / logo © 2022 Grokbase