8000 [Routing] extra parameters objects should be casted to string · Issue #26992 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Routing] extra parameters objects should be casted to string #26992

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
garak opened this issue Apr 20, 2018 · 13 comments · Fixed by #43612
Closed

[Routing] extra parameters objects should be casted to string #26992

garak opened this issue Apr 20, 2018 · 13 comments · Fixed by #43612
Labels
Feature Routing

Comments

@garak
Copy link
Contributor
garak commented Apr 20, 2018
Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? no
Symfony version any

Real case scenario: I want to pass an extra parameter to a route (see docs), and the value of such parameter is an object. The typical example is a Uuid object (e.g. Ramsey\Uuid).
Problem is that PHP is not smart enough and it does not cast objects to string. See related bug on php.net.
Of couse, such feature could be easily obtained in a typical plain PHP context, since it's very easy to cast your object to a string.
In a typical Symfony context, instead, you can't easily cast strings, unless doing ugly things.

Example of not working code:

{# generated URL has no query string #}
<a href="{{ path('my_route', {foo: object.uuid}) }}">my link</a>

Example of working code, with ugly workaround:

{# generated URL has expected query string, e.g. ?foo=7ecff38e-4145-11e8-80b9-0242ac140004 #}
<a href="{{ path('my_route', {foo: object.uuid.__toString}) }}">my link</a>

My proposal is: inside doGenerate method of UrlGenerator class, Symfony should cast parameters to string before passing them to http_build_query
@garak garak changed the title [Routing] [Routing] extra parameters not working with objects Apr 20, 2018
@garak garak changed the title [Routing] extra parameters not working with objects [Routing] extra parameters objects should be casted to string Apr 20, 2018
@linaori
Copy link
Contributor
linaori commented Apr 20, 2018

I don't think this is something you want. If an object has a __toString() which can be used for debugging purposes, or in case of the Request object, you might send sensitive data into the URL.

If you want this particular feature, please check out: https://github.com/iltar/http-bundle

@garak
Copy link
Contributor Author
garak commented Apr 20, 2018

@iltar it looks like your bundle serves a different purpose. Thanks for your suggestion anyway

@linaori
Copy link
Contributor
linaori commented Apr 20, 2018

I transforms an object to it's (string) representation, but with configuration rather than convention.

@garak
Copy link
Contributor Author
garak commented Apr 20, 2018

Maybe you missed the point when I explain I need this for extra route parameters

@linaori
Copy link
Contributor
linaori commented Apr 21, 2018 •
edited
Loading

That will just work, all the bundle does, is convert objects to scalar values, regardless of what's defined in the Route definition.

I haven't configured it in a while, because most cases are entities for me where the PK is used, but this should work

iltar_http:
    router:
        mapped_properties:
            Ramsey\Uuid: __toString
/** @Route("/", name="index") */
$urlGenerator->generate('index', ['foo' => $uuid]);
#Should generate: ?foo=7ecff38e-4145-11e8-80b9-0242ac140004

If this is not the case, I should probably fix this. When I have time, I'll add a test to verify this behavior.

Edit: I've seen this feature request come by quite often. If desired, I can also try to incorporate it into Symfony, or anyone can for that matter.

@Simperfit
Copy link
Contributor

@linaori do you have time to implement this ?

@linaori
Copy link
Contributor
linaori commented Apr 17, 2019

I can help if needed, don't have time to work on this myself. Any desired code can be grabbed from https://github.com/linaori/http-bundle and used. When implemented, I can deprecate this package and refer to symfony

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@carsonbot
Copy link

Friendly ping? Should this still be open? I will close if I don't hear anything.

@garak
Copy link
Contributor Author
garak commented Jan 2, 2021

Issue still stands

@carsonbot carsonbot removed the Stalled label Jan 2, 2021
@garak garak mentioned this issue Mar 2, 2021
Closed
@Tobion
Copy link
Contributor
Tobion commented Mar 5, 2021 •
edited
Loading

Looks like a duplicate of #10395.
To me, casting to string explicitly in user-land is the better solution.

Also the routing component does not deal with objects currently. It also does not support objects as route names (#37062). So adding this only for the params would be inconsistent.

@garak
Copy link
Contributor Author
garak commented Mar 5, 2021

It's not a duplicate.
The routing component is already casting objects to string, when objects are parameters used as placeholders.
I'm not asking for magic here, I'm just asking for consistency.
I'm posting my example again:

<a href="{{ path('my_route', {foo: object.uuid}) }}">my link</a>

It's working if route is something like /path/{foo} (uuid is cast to string)
It's not working if route is something like /path.
And, even worse, it's failing silently, you don't even get ?foo= in your URL.

@fabpot
Copy link
Member
fabpot commented Mar 5, 2021

Closing as explained in the related PR.

@fabpot fabpot closed this as completed Mar 5, 2021
@garak garak mentioned this issue Mar 7, 2021
Merged
javiereguiluz added a commit to symfony/symfony-docs that referenced this issue Apr 16, 2021
@javiereguiluz
minor #15069 [Routing] add a warning about object as extra parameter …
5e0c67a 
…in route (garak)

This PR was merged into the 4.4 branch.

Discussion
----------

[Routing] add a warning about object as extra parameter in route

As explained in [this issue](symfony/symfony#26992), objects are not converted to string when used as extra parameter.
I think that user should be warned about such behaviour, because failing to casting object is resulting in an empty query string, without any notice/warning/error.

Commits
-------

070c9bb add a warning about object as extra parameter in route
@nicolas-grekas nicolas-grekas mentioned this issue Jul 12, 2021
Closed
@nicolas-grekas nicolas-grekas mentioned this issue Oct 20, 2021
Merged
Tobion added a commit that referenced this issue Oct 26, 2021
@Tobion
minor #43612 [BrowserKit][HttpClient][Routing] support building query…
a76d746 
… strings with stringables (nicolas-grekas, OskarStark)

This PR was merged into the 5.4 branch.

Discussion
----------

[BrowserKit][HttpClient][Routing] support building query strings with stringables

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes (minor)
| Deprecations? | no
| Tickets       | Fix #26992
| License       | MIT
| Doc PR        | -

Allows using eg an instance of `Uid` as a route parameter.

Replaces #42057

Commits
-------

65e2cac Add check and tests for public properties
ab38bb8 [BrowserKit][HttpClient][Routing] support building query strings with stringables
@wiese wiese mentioned this issue Oct 27, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Routing
Projects
None yet
Milestone
No milestone
7 participants
@fabpot @garak @Tobion @linaori @xabbuh @Simperfit @carsonbot
0