Jonathan, >From sfSslRequirementActionMixin.class.php:
// return $security[$actionName]['ssl_domain'].$action->getRequest()- >getScriptName().$action->getRequest()->getPathInfo(); I don't think these changes take into account the value of the no_script_name config setting. The SSL switching includes the script name in the generated SSL URL regardless of what no_script_name is set to. Perhaps there should be a test for this? In a production environment when no_script_name is set to on and the page redirects to a secure connection "index.php" appears inside the URL. Regards, Marli On Jul 5, 12:45 am, "Jonathan H. Wage" <[EMAIL PROTECTED]> wrote: > Fabien, > > I have committed these changes. Here is an overview of what I have done. > > 1.) Added 2 new functions to sfComponent via mixins: getSslUrl() and > getNonSslUrl() > 2.) You can now specify in security.yml the non_ssl_domain and > ssl_domain > 3.) sslAllowed() now returns true if ssl is required. > > I have also created a ticket for the sfWebRequest isSecure() issue: > http://trac.symfony-project.com/trac/ticket/1931 > > I believe that is it. > > Thanks, Jon > > On Jul 4, 2007, at 12:09 AM, Fabien POTENCIER wrote: > > > > > Hi Jon, > > > I have little time to review those changes, so if everyone else is ok, > > please, go ahead and commit your changes. > > > As far as the isSecure() method is concerned, you can create a > > ticket. I > > think we can just add a new case in the exeisting condition: > > > return ( > > (isset($pathArray['HTTPS']) && (strtolower($pathArray['HTTPS']) > > == 'on' || strtolower($pathArray['HTTPS']) == 1)) > > || > > (isset($pathArray['HTTP_X_FORWARDED_PROTO']) && > > strtolower($pathArray['HTTP_X_FORWARDED_PROTO']) == 'https') > > ); > > > Thanks, > > Fabien > > > Jonathan H. Wage wrote: > >> Marli, > > >> I have all these things fixed, I am waiting to hear comments from > >> Fabien on whether or not I can commit them. I will let you know once > >> they are committed. > > >> - Jon > > >> On Jul 3, 2007, at 9:53 PM, Marli wrote: > > >>> Hey guys, thanks for the quick reply. I realise this is a bit late > >>> but here's my thoughts anyway: > > >>>> Would you want to specify the ssl and non ssl url for each > >>>> action? or > >>>> globally for the entire plugin? > >>> Globally for the entire plugin. It seems a bit over the top to > >>> have a > >>> different domain for every action. It's a minor change - instead of > >>> switching the "http" or "https" at the front of every URL, switch > >>> the > >>> entire domain as well to whatever is specified in config. > > >>> This is my (pretty ugly) change to > >>> sfSslRequirementFilter.class.php to > >>> allow this to happen: > > >>> // $controller->redirect(substr_replace($request->getUri(), > >>> 'http', 0, > >>> 5)); > > >>> // We want to change the request URL to use the non-SSL URL > >>> $controller->redirect(sfConfig::get('app_ssl_normal_url'). > >>> $_SERVER['PHP_SELF']); > > >>> It would be good if it didn't use $_SERVER - I wasn't sure how > >>> else to > >>> get the rest of the URL - but it works. I'm sure my config > >>> parameters > >>> are set in a bad place too (I was integrating it with what I already > >>> had). > > >>> Just a side note, my attempt doesn't do it but I think it should > >>> default back to the old behaviour if no URLs are specified, just > >>> changing the http/https as it does now. > > >>>> Maybe we should just make the plugin return true for allowSsl() if > >>>> require_ssl is set to true. It is kind of assumed that if you > >>>> want to > >>>> require_ssl, that you should allow it. > >>> I think that would be a wise idea. At the moment, if you don't > >>> specify > >>> allow_ssl, it causes an infinite redirect loop, which is totally > >>> unintuitive. If ssl is required, obviously it is allowed, it's basic > >>> transitivity. > > >>>> one not on allow_ssl default to false though: > >>>> you generally do not want people to use ssl where you dont > >>>> explicitly > >>>> enable it. it slows down your webserver .. less caching, > >>>> encryption work > >>>> etc. > >>> I agree, but allow_ssl can still default to false - just if > >>> require_ssl is true, allow_ssl is also true. They are not mutually > >>> exclusive. > > >>> Hope this helps, > > >>> (now if I can just find someone to fix that urlencoding annoyance in > >>> routing.... :-) > > >>> Marli > > >>> On Jul 4, 10:04 am, "Jonathan H. Wage" <[EMAIL PROTECTED]> wrote: > >>>> I will wait to hear what Fabien has to say about my changes. > > >>>> Thanks, Jon > > >>>> On Jul 3, 2007, at 5:27 PM, Lukas Kahwe Smith wrote: > > >>>>> On 03.07.2007, at 22:47, Jonathan H. Wage wrote: > >>>>>> Lukas, > >>>>>> I have a few fixes for the plugin, which have addressed all the > >>>>>> issues for myself, and the issues everyone else has talked > >>>>>> about on > >>>>>> the list. > >>>>>> You don't have to specify allow_ssl: true when require_ssl: > >>>>>> true is > >>>>>> set, and you can specify the ssl_domain and non_ssl_domain in the > >>>>>> security.yml. > >>>>>> Do you think I should just commit these fixes or submit a patch? > >>>>> well fabian told me to just commit my fixes when i first came with > >>>>> some patches .. > >>>>> its not my plugin ... > >>>>> regards, > >>>>> Lukas --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "symfony developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/symfony-devs?hl=en -~----------~----~----~----~------~----~------~--~---
