fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

generate_base_url misses trailing slash if script_name is empty #863

Closed mikebranderhorst closed 12 years ago

mikebranderhorst commented 12 years ago

If somehow script_name is empty, base_url misses a trailing slash.

https://github.com/fuel/core/blob/1.1/develop/classes/fuel.php#L308

    /**
     * Generates a base url.
     *
     * @return  string  the base url
     */
    protected static function generate_base_url()
    {
        $base_url = '';
        if(\Input::server('http_host'))
        {
            $base_url .= \Input::protocol().'://'.\Input::server('http_host');
        }
        if (\Input::server('script_name'))
        {
            $base_url .= str_replace('\\', '/', dirname(\Input::server('script_name')));

            // Add a slash if it is missing
            $base_url = rtrim($base_url, '/').'/';
        }
        return $base_url;
    }

Shouldn't this be the correct code?


    /**
     * Generates a base url.
     *
     * @return  string  the base url
     */
    protected static function generate_base_url()
    {
        $base_url = '';

        if (\Input::server('http_host'))
        {
            $base_url .= \Input::protocol().'://'.\Input::server('http_host');
        }

        if (\Input::server('script_name'))
        {
            $base_url .= str_replace('\\', '/', dirname(\Input::server('script_name')));
        }

        // Add a slash if it is missing
        return rtrim($base_url, '/').'/';
    }
WanWizard commented 12 years ago

Script name shouldn't be empty. If it is, your base_url will never by correct, so returning a slash doesn't do you much good.

What environment are you using (fgci perhaps?), and is it available elsewhere (like in $ENV)?

jschreuder commented 12 years ago

This has waited 8 days, if it gets no further feedback it will be closed.

jschreuder commented 12 years ago

Closing due to no feedback.

mikebranderhorst commented 12 years ago

Thx for feedback, I can't reproduce the problem (sorry) and the system I use for dev is MAMP 2.0.5 default settings.

jgivoni commented 12 years ago

I would like to reopen this issue.

I have the same problem, and I can reproduce it in the following setup:

When Apache server is configured with an AliasMatch to route all requests (except /assets/) to index.php, the 'script_name' for some reason do not get set:

Alias /assets/ /var/www/fuelphp/public/assets/ AliasMatch ^(.*)$ /var/www/fuelphp/public/index.php/$1

I have no idea whether this is a bug in Apache (2.2.17), but in any case it seems undue to check for trailing slash inside the conditional.

Rather than insist that script_name should never be empty for fuelphp to work, wouldn't it be better to simply add the trailing slash, and thereby making it work for people who prefer Alias over Rewrite rules?

Thanks, Jakob