search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
721 stars 821 forks source link

Controller::join_links is breaking url encoding for multiple url params #3042

Closed jennymgray closed 10 years ago

jennymgray commented 10 years ago

This seems to be as a result of commit https://github.com/silverstripe/silverstripe-framework/commit/a8c1dd7f535bce772a06c9717b09f2ba882300db

I identified this with an RSSFeed where the $Link passed is being double-encoded by the template engine.

My $Link should output as
***?format=rss&type=liked&token=825392989843169135 (full url details removed for my security).

So in the past, I have passed in the fully encoded url, as above, as suggested by the comments for the Controller::join_links() function like this:

$feed = new RSSFeed( $annotations, 'feed??format=rss&type=liked&token=825392989843169135', $title, 'OU Annotate', 'ItemTitle', 'ItemDescription', 'ItemCreator', time() );

However, now the link is output as ***?format=rss&3B;type=liked&3B;token=825392989843169135

I believe the solution is that on line 578 the http_build_query call should be

http_build_query($queryargs, ' ' , '&')

And urls should be passed in UNencoded.

I am nervous however, that this could have effects elsewhere in the system that I don't use. I suppose the alternative would be to backtrack from http_build_query, but you must have had a reason for swapping to that in the first place!

micmania1 commented 10 years ago

You have 2 question marks in your link which might be breaking it.

jennymgray commented 10 years ago

Ah sorry, no - that's a copy & paste error as I wrote the bug report out, rather than part of the problem.

PonchoPowers commented 10 years ago

In the Controller (framework/control/Controller.php)

After the following line: if($queryargs) $result .= '?' . http_build_query($queryargs);

Add the following: elseif (isset($suffix)) $result .= '?';

Because the $suffix can be a '?' with the intention of postfixing after it.

jennymgray commented 10 years ago

That may help some-one else's problem but not mine. In my situation, $queryargs is set incorrectly, so you are in the if part of the code. Adding an elseif doesn't work because it isn't called. As I said in the original thread, changing like this works for me ...

if ($queryargs) $result .- '?' . http_build_query($queryargs, ' ' , '&')

tractorcow commented 10 years ago

Have you perhaps got an odd character specified for your arg-separator.output config?

jennymgray commented 10 years ago

Nope, it's an & as you would expect.

However, I started this thread a few months back, so with your recent input I decided to go back to basics on the bug and see if I could work out why you can't replicate it. I think I've found a problem with the content I'm passing in as the RSSFeed $link and its encoding which is the root of the issue.

Thanks for helping!