merged branch gimler/master (PR #1894)

Commits
-------

86f888f fix https default port check

Discussion
----------

fix https default port check

---------------------------------------------------------------------------

by Abhoryo at 2011/08/03 03:26:15 -0700

I think it's better to delete $httpsPort variable from the prototype and use only $httpPort variable.

public function urlRedirectAction($path, $permanent = false, $scheme = null, $httpPort = 80)
...
        $port = '';
        if (('http' === $scheme && 80 != $httpPort)  || ('https' === $scheme && 443 != $httpPort)) {
            $port = ':'.$httpPort;
        }

But if this method is already used with the $httpsPort variable elsewhere, your change is ok with me.

---------------------------------------------------------------------------

by gimler at 2011/08/03 04:52:08 -0700

You can use different ports for http and https so when you call the function $scheme = null than it use the $request->getScheme() so you must add both ports so i think it is not a good idea to merge the http and https vars.

---------------------------------------------------------------------------

by gimler at 2011/08/03 04:53:17 -0700

damn sorry i have accidentally close the pull request ;(

---------------------------------------------------------------------------

by stof at 2011/08/03 05:13:24 -0700

I agree with @gimler. Merging them as a single parameter does not make sense here

---------------------------------------------------------------------------

by Abhoryo at 2011/08/03 05:33:12 -0700

I've juste think it's weird to set a useless parameter ($httpPort) when you want to use the last parameter ($httpsPort).
And I don't think someone want http protocole on 433 or https on 80 ?

---------------------------------------------------------------------------

by stof at 2011/08/03 05:35:16 -0700

@Abhoryo what if you are using this controller in a general way, without knowing by advance if the handled request is a secure one ? You need both parameters.
If you need to change the https port by keeping the default http port, you indeed need to pass it but blame PHP: it does not support named parameters.

---------------------------------------------------------------------------

by Abhoryo at 2011/08/03 06:02:18 -0700

Ok, right.

src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php

@@ -74,6 +74,7 @@ class RedirectController extends ContainerAware
         if (null === $scheme) {
             $scheme = $request->getScheme();
         }
+
         $qs = $request->getQueryString();
         if ($qs) {
             $qs = '?'.$qs;
@@ -82,7 +83,7 @@ class RedirectController extends ContainerAware
         $port = '';
         if ('http' === $scheme && 80 != $httpPort) {
             $port = ':'.$httpPort;
-        } elseif ('https' === $scheme && 443 != $httpPort) {
+        } elseif ('https' === $scheme && 443 != $httpsPort) {
             $port = ':'.$httpsPort;
         }