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

Missing proxy-headers option like one in gunicorn #105

Closed
dragoangel opened this issue Feb 28, 2023 · 16 comments
Closed

Missing proxy-headers option like one in gunicorn #105

dragoangel opened this issue Feb 28, 2023 · 16 comments

Comments

@dragoangel
Copy link

dragoangel commented Feb 28, 2023

Currently, it's impossible to get a properly working service behind a proxy as headers like x-forwarded-for, x-forwarded-proto, x-forwarded-host, x-forwarded-port and x-forwarded-prefix isn't checked. for and proto most critically needed, as they used in logging, application behaviors (including critical one, f.e.: return metrics if trusted_ips(client)) and URL generation (f.e.: url_for).

Can this be checked, please?

@Kludex
Copy link

Kludex commented Mar 10, 2023

@pgjones Would you like me to work on this? I'm actually improving the ProxyHeadersMiddleware on Uvicorn right now.

I guess the idea here would be to populate the scope["client"], scope["host"] and scope["scheme"].

@sinwoobang
Copy link

@dragoangel Is your problem happening when you implement a server with hypercorn only? Otherwise, is it still same though you use other frameworks such as Django or Flask?

@dragoangel
Copy link
Author

I don't use Flask or Django, so can't say. I used uvicorn (and gunicorn) with Starlette and now switched to hypercorn with Starlette.

@sinwoobang
Copy link

sinwoobang commented Mar 14, 2023

@dragoangel Does it mean that you cannot access the raw Proxy headers on your Starlette application?
Otherwise, do you want a native-support where Hypercorn detects proxy headers and take proper actions?

@dragoangel
Copy link
Author

@dragoangel Does it mean that you cannot access Proxy headers on your Starlette application?

It's mean I not have options to get proper client ip, scheme, host on default variables while behind proxy and I don't have a way to set trusted proxy. Using proxy should be transparent for code when used with proxy milter, e.g.:

  • no proxy, no milter, same code if (client)...
  • behind proxy, configured milter, same code if (client)...

@jackboy2fly
Copy link

Is this something in the plans to get added? I am running behind a proxy with FastAPI and having issues with redirect URLs not using the x-forwarded-proto.

@gellnerm
Copy link

I switched back to uvicorn because of the --proxy-headers being missing in hypercorn. I need to be able to get the request.client IP behind a proxy.

@Kludex
Copy link

Kludex commented Dec 23, 2023

You don't need to switch back. You can use the ProxyHeadersMiddleware from uvicorn, and continue to use use hypercorn.

@dragoangel
Copy link
Author

dragoangel commented Dec 23, 2023

You don't need to switch back. You can use the ProxyHeadersMiddleware from uvicorn, and continue to use use hypercorn.

How can it be achieved when used as hypercorn+starlette only?

It truly works as:

...
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware
...
proxy_hosts = os.getenv('PROXY_HOSTS', '127.0.0.1').split(",")
app = Starlette()
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=proxy_hosts)
...

But I think importing another package is really bad just to get one basic middleware from it, right?
Proxy thing is common and should be part of hypercorn - this is my point.

@gellnerm
Copy link

You don't need to switch back. You can use the ProxyHeadersMiddleware from uvicorn, and continue to use use hypercorn.

Can we have documentation for that?

@dragoangel
Copy link
Author

You don't need to switch back. You can use the ProxyHeadersMiddleware from uvicorn, and continue to use use hypercorn.

Can we have documentation for that?

I don't think this is an acceptable solution in the long term, it looks more like workaround of the issue and not the solution.

@pgjones
Copy link
Owner

pgjones commented Dec 28, 2023

See the ProxyFixMiddleware added in 4fc0372

@pgjones pgjones closed this as completed Dec 28, 2023
@dragoangel
Copy link
Author

Hi @pgjones , thank you :)

@dragoangel
Copy link
Author

dragoangel commented Jan 4, 2024

@pgjones in my setup it results in 500 and throws error in logs like:

[2024-01-04 15:28:12 +0000] [14] [ERROR] Error in ASGI Framework
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/hypercorn/asyncio/task_group.py", line 27, in _handle
    await app(scope, receive, send, sync_spawn, call_soon)
  File "/usr/local/lib/python3.10/site-packages/hypercorn/app_wrappers.py", line 34, in __call__
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.10/site-packages/starlette/applications.py", line 116, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/usr/local/lib/python3.10/site-packages/starlette/middleware/errors.py", line 186, in __call__
    raise exc
  File "/usr/local/lib/python3.10/site-packages/starlette/middleware/errors.py", line 164, in __call__
    await self.app(scope, receive, _send)
  File "/usr/local/lib/python3.10/site-packages/starlette_exporter/middleware.py", line 268, in __call__
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.10/site-packages/starlette/middleware/gzip.py", line 26, in __call__
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.10/site-packages/asgi_correlation_id/middleware.py", line 90, in __call__
    await self.app(scope, receive, handle_outgoing_request)
  File "/usr/local/lib/python3.10/site-packages/hypercorn/middleware/proxy_fix.py", line 22, in __call__
    scope = deepcopy(scope)
  File "/usr/local/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/local/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/local/lib/python3.10/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/local/lib/python3.10/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/local/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/local/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/local/lib/python3.10/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/local/lib/python3.10/copy.py", line 272, in _reconstruct
    if hasattr(y, '__setstate__'):
  File "/usr/local/lib/python3.10/site-packages/starlette/datastructures.py", line 709, in __getattr__
    return self._state[key]
  File "/usr/local/lib/python3.10/site-packages/starlette/datastructures.py", line 709, in __getattr__
    return self._state[key]
  File "/usr/local/lib/python3.10/site-packages/starlette/datastructures.py", line 709, in __getattr__
    return self._state[key]
  [Previous line repeated 966 more times]
RecursionError: maximum recursion depth exceeded

I added it as:

from hypercorn.middleware import ProxyFixMiddleware
from asgi_correlation_id import CorrelationIdMiddleware, correlation_id
from starlette.applications import Starlette
from starlette.middleware.gzip import GZipMiddleware

proxy_hops = int(os.getenv('PROXY_HOPS', '0'))
proxy_mode = os.getenv('PROXY_MODE', 'legacy')

app = Starlette()
app.add_middleware(ProxyFixMiddleware, mode=proxy_mode, trusted_hops=proxy_hops)
app.add_middleware(CorrelationIdMiddleware, header_name='X-Request-ID')
app.add_middleware(GZipMiddleware, minimum_size=1000)

@gellnerm
Copy link

gellnerm commented Jan 4, 2024

@pgjones in my setup it results in 500 and throws error in logs like:

It due to #179
As a workaround, add it like described here: 4fc0372

@dragoangel
Copy link
Author

@pgjones in my setup it results in 500 and throws error in logs like:

It due to #179 As a workaround, add it like described here: 4fc0372

Thank you for linking an issue :)

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

No branches or pull requests

6 participants