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

No way to use xheaders in tornado worker #2156

Open
spanezz opened this issue Nov 8, 2019 · 1 comment
Open

No way to use xheaders in tornado worker #2156

spanezz opened this issue Nov 8, 2019 · 1 comment

Comments

@spanezz
Copy link

spanezz commented Nov 8, 2019

Thanks for gunicorn! I am using it to deploy a tornado server using the tornado worker.

Deployment is behind a proxy, so I need to activate xheaders in the tornado httpserver, and use --forwarded-allow-ips in gunicorn.

In workers/gtornado.py, however, I see no way to set xheaders=True in the created tornado httpserver. This seems odd because in the traceback in #783 there seems to be or have been code for it.

My first (failed) attempt was using a python configuration with this hook:

	def post_worker_init(worker):
	    worker.server.xheaders = True

however, at the time the hook is called, worker.server is not filled,
and it is only filled in the TornadoWorker.run() method, which does not
terminate and so cannot be extended. There seem to be no obvious way to
hook into tornado.httpserver.HTTPServer creation inside TornadoWorker.

I found a non-obvious way: monkey patching tornado.httpserver.HTTPServer
to default to xheaders=True:

	def post_worker_init(worker):
	    import tornado.httpserver

	    class XheadersServer(tornado.httpserver.HTTPServer):
		def __init__(self, *args, **kw):
		    super().__init__(*args, **kw)
		    self.xheaders = True

	    tornado.httpserver.HTTPServer = XheadersServer

This way it seems to work.

Possible ways of making this easier could be to refactor the tornado worker class a bit to be able to customize the tornado HTTPServer object in post_worker_init before it is run.

Alternatively, one can have a way to configure the tornado worker with a separate class name for the HTTPServer.

Alternatively, one can set xheaders=True whenever --forwarded-allow-ips is used; it feels to me like too much magic, though, and there may be other customizations to HTTPServer that one may want to do.

@tilgovi
Copy link
Collaborator

tilgovi commented Nov 23, 2019

Alternatively, one can set xheaders=True whenever --forwarded-allow-ips is used; it feels to me like too much magic, though, and there may be other customizations to HTTPServer that one may want to do.

Can Tornado support only specific IPs like Gunicorn for the xheaders flag? It looks like it's only a boolean value.

I don't think this kind of behavior is too much magic, unless it doesn't behave exactly correctly and then it's maybe risky and surprising.

Have you thought about it any more since posting?

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

No branches or pull requests

3 participants