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

Sinatra (and probably other Rack apps) do not properly generate links #22

Open
halfbyte opened this issue Aug 14, 2015 · 8 comments · Fixed by experimental-platform/platform-dokku#13
Assignees
Labels

Comments

@halfbyte
Copy link

I'll create a simple test case, but for now, here's the info I gathered:

When nginx proxies the request to the rack app, the sub folder path information gets lost and if you use to/url to generate proper url's you'll always end up with urls that are constructed from the root.

Let's say I created an app "example", and it runs at foo.local/example. Every url constructed with url() or to() in sinatra ends up being wrongly constructed, for example url('/foo') returns /foo instead of /example/foo.

I have not completely understood the underlying problem, as sinatra claims to support reverse proxying and sub folders. One issue seems to be that SCRIPT_NAME is empty, maybe adding that to the proxy headers could help.

As sinatra is a good fit for quick hacks, I consider this a problem :)

I'll update this issue as I gather more information.

@halfbyte
Copy link
Author

I have a workaround and a possible fix:

The workaround is to add "rack_injection" to the Gemfile and add this to the config.ru file of your sinatra app (replace $APP with the name of your app, of course):

require 'rack_injection'
use RackInjection, 'SCRIPT_NAME' => '$APP'

The fix for experimental platform would be to set SCRIPT_NAME as a header in the nginx config. I hope.

@halfbyte
Copy link
Author

Okay, next problem with url(): if running via the protonet proxy (foobar.protonet.info) the X_FORWARDED_PROTO header is not correctly set (I assume it gets lost in the go request router?) which generates http links which makes the protonet proxy redirect to https.

And here I was wondering how somehow magically POST requests were turned into GET requests :)

@tiff
Copy link
Member

tiff commented Aug 14, 2015

Thanks @halfbyte we will look into this as soon as possible. The URL issue is something that's very high on our todo list :)
The X_FORWARDED_PROTO fix seems like a low hanging fruit that might be fixed even earlier. I'll keep you posted.

@tiff tiff added the bug label Aug 14, 2015
@tiff tiff added this to the 20150826 milestone Aug 14, 2015
@tiff tiff self-assigned this Aug 14, 2015
tiff added a commit to experimental-platform/platform-dokku that referenced this issue Aug 27, 2015
@tiff
Copy link
Member

tiff commented Aug 27, 2015

@halfbyte experimental-platform/platform-dokku#13 this should fix the X_FORWARDED_PROTO issue. Would this also be the place to put the SCRIPT_NAME or shouldn't this rather be an environment variable?

@tiff
Copy link
Member

tiff commented Aug 31, 2015

I would appreciate your feedback @halfbyte 😃

@halfbyte
Copy link
Author

@tiff I think the nginx config would be the correct place to set the SCRIPT_NAME, yes. Basically, the SCRIPT_NAME must match the path you mount the sinatra app at, so, if the app name is jinglebells, the SCRIPT_NAME should be jinglebells as well, if the app is then to be found at mybox.protonet.info/jinglebells.

I'm not entirely sure it's the correct fix for my problem, but it will be worth a try.

@tiff tiff reopened this Sep 3, 2015
@tiff tiff removed this from the 2015-09-03 milestone Sep 3, 2015
@kdomanski
Copy link
Contributor

@halfbyte can I get some sample code to reproduce the issue?

@tiff
Copy link
Member

tiff commented Nov 2, 2015

A lot of popular frameworks only run when they are in the root path (domain.com/).

In the future we will make apps available as multi level subdomains (app-name.box-name.protonet.info). The only challenge we have is that we would need valid dynamically created ssl certificates for that.
Let's Encrypt makes it possible. We received our beta access last week and are already digging into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants