Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accept implicit port 80 on whitelist #3513

Merged
merged 2 commits into from Jan 4, 2016
Merged

accept implicit port 80 on whitelist #3513

merged 2 commits into from Jan 4, 2016

Conversation

bryevdv
Copy link
Member

@bryevdv bryevdv commented Jan 3, 2016

Reference: request/request#515

Many programs strip the port from the host header when it is 80, and this breaks our whitelist checking. This prevents the server from being run behind a proxy on port 80.

@havocp
Copy link
Contributor

havocp commented Jan 3, 2016

lgtm. it might not hurt to test a couple actual http requests (I added stuff to test_server.py which should make that simple now). In case tornado ever does something unexpected before we see the Host value or something.

@havocp
Copy link
Contributor

havocp commented Jan 3, 2016

lgtm. could add a websocket test for this case

@bryevdv
Copy link
Member Author

bryevdv commented Jan 3, 2016

I tested this on a locally running nginx with this configuration:

worker_processes auto;

error_log   /tmp/error.log;
pid         /tmp/nginx.pid;

events {
    worker_connections  1024;
}

http {
    default_type    application/octet-stream;

    sendfile            on;
    tcp_nopush          on;
    tcp_nodelay         on;

    keepalive_timeout   65;
    types_hash_max_size 2048;

    server {
        listen 80 default_server;
        server_name _;

        access_log  /tmp/bokeh.access.log;
        error_log   /tmp/bokeh.error.log  debug;

        location /apps/ {
            proxy_pass http://127.0.0.1:5100;
            proxy_set_header Upgrade $http_upgrade;
            proxy_set_header Connection "upgrade";
            proxy_http_version 1.1;
            proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
            proxy_set_header Host $host:$server_port;
            proxy_buffering off;
        }

        location /static {
            alias /Users/bryan/work/bokeh/bokeh/server/static;
        }
    }
}

And running the server with:

bokeh serve sliders.py  --prefix=apps/ --address=127.0.0.1 --port 5100 --host localhost:80

@bryevdv
Copy link
Member Author

bryevdv commented Jan 3, 2016

I'm using the same whitelist checker in both cases, and the checker has tests, is that sufficient?

bryevdv added a commit that referenced this pull request Jan 4, 2016
accept implicit port 80 on whitelist
@bryevdv bryevdv merged commit 403e6a8 into master Jan 4, 2016
@bryevdv bryevdv added this to the 0.11.0 milestone Jan 4, 2016
@damianavila damianavila deleted the request_port_80 branch January 4, 2016 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants