Mailing List Archive: uri_for() and URI fragments

uri_for() and URI fragments

Aug 27, 2010, 6:45 AM

Post #1 of 5 (3488 views)

I have an app that is currently running on Catalyst 5.7007, which I am
working on migrating to the current version. I have discovered a
regression in how uri_for() handles URI fragments (the part of the URI
after a #).

I have a call to uri_for() like this

$c->uri_for(
'/user/' . $username . '#comments',
{request_comment_id => $comment_id},
);

With 5.7007, the resulting URI ends:
/user/soandso?request_comment_id=1234#comments

But with a newer Catalyst, it ends:
/user/soandso#comments?request_comment_id=1234

5.7007 used the URI module to create the URI. Starting in 5.7008 [*]
a big block of custom code is used instead. While this provides
better performance, it is much less reliable.

Is this a bug that should be fixed, or is there a different, correct
way of specifying a URI fragment with uri_for?

[*] In another quirk, the changelog incorrectly lists this change for
5.7007; it actually changed in 5.7008.

Ronald

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev@lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

Re: uri_for() and URI fragments [ In reply to ]

bobtfish at bobtfish

Sep 1, 2010, 3:20 PM

Post #2 of 5 (3392 views)

Permalink

On 27 Aug 2010, at 14:45, Ronald J Kimball wrote:
> 5.7007 used the URI module to create the URI. Starting in 5.7008 [*]
> a big block of custom code is used instead. While this provides
> better performance, it is much less reliable.
>
>
> Is this a bug that should be fixed, or is there a different, correct
> way of specifying a URI fragment with uri_for?

No, it's almost certainly a bug.

Any chance of a failing test case?

Cheers
t0m

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev@lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

Re: uri_for() and URI fragments [ In reply to ]

rkimball at pangeamedia

Sep 3, 2010, 6:07 AM

Post #3 of 5 (3395 views)

Permalink

On Wed, Sep 1, 2010 at 6:20 PM, Tomas Doran <bobtfish@bobtfish.net> wrote:
>
> On 27 Aug 2010, at 14:45, Ronald J Kimball wrote:
>>
>> Is this a bug that should be fixed, or is there a different, correct
>> way of specifying a URI fragment with uri_for?
>
> No, it's almost certainly a bug.
>
> Any chance of a failing test case?

This test should be added to t/aggregate/unit_core_uri_for.t:

is(
Catalyst::uri_for( $context, '/bar#anchor', { param1 => 'value1' }
)->as_string,
'http://127.0.0.1/foo/bar?param1=value1#fragment',
'URI for path with fragment and query params'
);

# Failed test 'URI for path with fragment and query params'
# at t/aggregate/unit_core_uri_for.t line 61.
# got: 'http://127.0.0.1/foo/bar#anchor?param1=value1'
# expected: 'http://127.0.0.1/foo/bar?param1=value1#fragment'

Ronald

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev@lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

Re: uri_for() and URI fragments [ In reply to ]

uwe at uwevoelker

Sep 3, 2010, 7:12 AM

Post #4 of 5 (3385 views)

Permalink

Did you mix up #anchor and #fragment?

Uwe

> This test should be added to t/aggregate/unit_core_uri_for.t:
>
> is(
> Catalyst::uri_for( $context, '/bar#anchor', { param1 => 'value1' }
> )->as_string,
> 'http://127.0.0.1/foo/bar?param1=value1#fragment',
> 'URI for path with fragment and query params'
> );
>
>
> # Failed test 'URI for path with fragment and query params'
> # at t/aggregate/unit_core_uri_for.t line 61.
> # got: 'http://127.0.0.1/foo/bar#anchor?param1=value1'
> # expected: 'http://127.0.0.1/foo/bar?param1=value1#fragment'

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev@lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

Re: uri_for() and URI fragments [ In reply to ]

rkimball at pangeamedia

Sep 3, 2010, 7:19 AM

Post #5 of 5 (3395 views)

Permalink

2010/9/3 Uwe Völker <uwe@uwevoelker.de>:
> Did you mix up #anchor and #fragment?

I did, thank you.

Here's a corrected test:

is(
Catalyst::uri_for( $context, '/bar#fragment', { param1 => 'value1'
} )->as_string,
'http://127.0.0.1/foo/bar?param1=value1#fragment',
'URI for path with fragment and query params'
);

# Failed test 'URI for path with fragment and query params'
# at t/aggregate/unit_core_uri_for.t line 61.
# got: 'http://127.0.0.1/foo/bar#fragment?param1=value1'
# expected: 'http://127.0.0.1/foo/bar?param1=value1#fragment'

Ronald

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev@lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev