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

Configure absolute and server URL prefixes #85

Merged
merged 10 commits into from
Jan 30, 2019

Conversation

manics
Copy link
Member

@manics manics commented Jan 16, 2019

This started off as a fix for the X-Forwarded-Context X-ProxyContextPath. Previously they were always /proxy/{port}, now they should be /server-name if this is a managed server. Since this is related to the code that controls the URL rewriting I thought I'd add in support for absolute URLs.

I've done some testing with:

c.ServerProxy.servers = {
    'python-http': {
        'command': ['python3', '-m', 'http.server', '{port}'],
    },
    'python-http-abs': {
        'command': ['python3', '-m', 'http.server', '{port}'],
        'rewrite': '',
    },
}

python-http should produce logs like

"GET / HTTP/1.1" 200 -

python-http-abs should produce logs like

"GET /python-http-abs/ HTTP/1.1" 404 -

Also /proxy-abs/port is the absolute version of /proxy/port

Needs more testing! @jacobtomlinson @psychemedia

Closes #43

@manics
Copy link
Member Author

manics commented Jan 16, 2019

Last two commits add the changes suggested by @yuvipanda (#84 (comment)) and adds a travis test

manics added a commit to manics/jupyterlab that referenced this pull request Jan 20, 2019
@yuvipanda
Copy link
Contributor

@manics I've made the following changes here:

  1. Call it absolute_url and make it boolean, instead of rewrite
  2. Make it work with websockets too

This is an amazing contribution, thank you very much for it!

I'll merge shortly

@yuvipanda
Copy link
Contributor

@manics I'm actually going to wait for you to OK my changes before merging.

Copy link
Member Author

@manics manics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuvipanda I'm OK with your changes, just need to fix Travis.

jupyter_server_proxy/handlers.py Show resolved Hide resolved
@yuvipanda
Copy link
Contributor

Am also super thankful for adding tests :) Would be nice to switch to Circle at some point though.

@yuvipanda yuvipanda merged commit aad1ed9 into jupyterhub:master Jan 30, 2019
@yuvipanda
Copy link
Contributor

@manics I've made a new release (1.0.0b9) with your changes! \o/

Should really make a 1.0 release #86

ian-r-rose pushed a commit to ian-r-rose/jupyterlab that referenced this pull request Jan 30, 2019
@manics manics deleted the proxy-base-urls branch January 31, 2019 09:43
ian-r-rose pushed a commit to ian-r-rose/jupyterlab that referenced this pull request Feb 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doesn't work with applications which use absolute URLs
2 participants