Luracast / Restler

Simple and effective multi-format Web API Server to host your PHP API as Pragmatic REST and/or RESTful API
http://luracast.com/products/restler/
GNU Lesser General Public License v2.1
1.36k stars 316 forks source link

Incorrect $baseUrl problem & fix #175

Closed vedmaker closed 11 years ago

vedmaker commented 11 years ago

Restler 3.0.0rc3 with nginx hosting multiple virtual web servers does not work properly due to incorrect $baseUrl derived from SERVER_NAME environment variable in the Restler::getPath() function. Instead $baseUrl derived from HTTP_HOST environment variable works properly.

Arul- commented 11 years ago

Relying on HTTP_HOST is simply wrong as it totally depends on client to send it

Instead configure nginx to send SERVER_NAME properly

as given in the nginx example in https://github.com/Luracast/Restler/blob/v3/README.md

server {
        listen        80;
        server_name   api.luracast.com; //change it to match your server name

        //... other stuff

        location ~ \.php$ {
            root           /var/www/html;
            fastcgi_pass   127.0.0.1:9000;
            fastcgi_index  index.php;
            fastcgi_param  SCRIPT_FILENAME  /var/www/html/$fastcgi_script_name;
            include        fastcgi_params;
        }

        //... other stuff

}
vedmaker commented 11 years ago

Thanks for the explanation.

roynasser commented 11 years ago

Hi Arul, when using dynamic host names/wildcard domains, etc, can you recommend what would be the best practice way to go?

This is what I have in our getPath, but maybe there are other better practices? Since using offloading of SSL in the laodbalancer, _SERVER HTTPS header isnt set, so using SSL_CIPHER set by the ssl terminators made more sense... Comments?

    $fullPath = urldecode($_SERVER['REQUEST_URI']);
    $path = Util::removeCommonPath(
        $fullPath,
        $_SERVER['SCRIPT_NAME']
    );
    $baseUrl = isset($_SERVER['X-SSL-CIPHER']) ||
        $_SERVER['HTTPS'] == 'on' ? 'https://' : 'http://';
        $baseUrl .= $_SERVER['HTTP_HOST'];

/* if ($_SERVER['SERVER_PORT'] != '80') { $baseUrl .= $_SERVER['SERVER_NAME'] . ':' . $_SERVER['SERVER_PORT']; } else { $baseUrl .= $_SERVER['SERVER_NAME']; }*/ $this->baseUrl = $baseUrl . rtrim(substr( $fullPath, 0, strlen($fullPath) - strlen($path) ), '/');

Since we depend on the browser to supply an http request, including the requested host name, what is the harm in using the aforementioned supplied hostname to deliver the path? I mean it isnt like it is something that is "generated" by the server?

Arul- commented 11 years ago

on nginx http://nginx.org/en/docs/http/request_processing.html can help you get your server name right

It will internally use requests host header to match the right domain and pass that domain name to php

roynasser commented 11 years ago

Hi @Arul- , thanks for the link. Nonetheless, for an API which obtains account information from the domain, i.e. the server is configured to allow * (any domain, as long as it is pointed to the correct IP), and the API must receive the data and be able to treat it to serve the request, I still dont see how we can get away from using HTTP_HOST?

Thanks for clarifying

roynasser commented 11 years ago

I just upgraded to RC4 and noticed that the SERVER_NAME is coming up in the rsources (swagger)... I dont remember what was the final decision regarding the use of server vs http_host since there was some issue esp in cases when there is wildcard domains in use and nginx has no way of fixing the domain name on the server_name...

Is this a problem in RC4? Is it something that should be changed? (Im sure I can find where its going on and change it, but wanted to put it here so we can get a final fix for all webservrs and conditions - be it wildcard, be it fixed domain, etc...)

http://_:8081/API/resources/xxxxxx.json is what swagger is trying to load, and obviously this fails...

Changing the getPath method to set baseUrl simply by using:

$baseUrl = isset($_SERVER['HTTP_X_SSL_CIPHER']) || $_SERVER['HTTPS'] == 'on' ? 'https://' : 'http://'; $baseUrl .= $_SERVER['HTTP_HOST']; (which solves the issue also of loadbalancers that set the HTTP_X_SSL_CIPHER header and offload ssl termination) has solved the problem... Also since HTTP_HOST includes port and everything, no need to add extra code...

For now I am changing manually at each release, but it would be best to get this issue solved...

Arul- commented 11 years ago

You don't have to patch restler for this, instead simply update SERVER_NAME in your index.php before initialising restler

$_SERVER['SERVER_NAME'] =  $_SERVER['HTTP_HOST'];

ideally you may use some regular expression to check if the server name falls with in your wildcard possibilities

roynasser commented 11 years ago

This is a good idea... so i dont need to change restler code...

Our application already checks the domain is valid in index.php before entering API. I will also set HTTPS accordingly from X_SSL_CIPHER... 1 thing, I will need to do some str splitting, etc, because http_host will contain port, correct? This will be a good solution for me... I hope other ppl dont have a problem doing this ;)