fix EZP21591: querystring lost in redirection

[ghost]

will carry the querystring params on a redirect operation.

the QueryString is normalized by the request(), so i needed to manually add the '?' for the the fix to work. yes, looks ugly, is there a better way to do it?

[lolautruche]

+1

[andrerom]

+1 (but nitpick, what is "Carrier" in this context? better with just $querystring or something ?)

[ghost]

it's what the variable is for, it "carries" the querystring over the redirect op.
bytes are cheap, and so are keystrokes.... :)

[lolautruche]

@Pbras I agree with @andrerom on the nitpick. This is confusing. Can you please amend your commit ?

[patrickallaert]

CS: $queryString in camel caps.

But better would be to just not use a temporary variable and write directly:

$semanticPathinfo . '?' . $request->getQueryString()

to keep the context closer.

[lolautruche]

We're forced to use a temp variable here (for keeping readability). And it makes me thing that we don't have always a query string while we enforce it below.

[patrickallaert]

+1 but take other's remark into account.

[lolautruche]

What if $queryString is empty ? We'll have an orphan ?.

Better do something like this:

$queryString = $request->getQueryString();

// ...
new RedirectResponse ( $semanticPathinfo . $queryString ? "?$queryString" : '' );

[ezrobot]

This Pull Request does not respect our Coding Standards, please, see the report below:

FILE: ...ce/eZ/Bundle/EzPublishCoreBundle/EventListener/RequestEventListener.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 151 | ERROR | Space found before comma in function call
--------------------------------------------------------------------------------

[ghost]

good to go?

[lolautruche]

You have 3 "+1" so yes it is ok.
However, please squash your 2 commits into one (easier for backports).

[lolautruche]

Manually merged in 9a70edc