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

[BUG] Can't add/override HTTP headers in other middlewares #101

Open
alessiofachechi opened this issue Apr 18, 2023 · 5 comments
Open

[BUG] Can't add/override HTTP headers in other middlewares #101

alessiofachechi opened this issue Apr 18, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@alessiofachechi
Copy link

Describe the bug
Putting the middleware on top of middleware list — as suggested by doc — doesn't allow next middlewares to potentially add/override HTTP headers through:

request.META['HTTP_CUSTOM_HEADER'] = "CUSTOM VALUE"

This is due to using django_guid.utils.get_id_from_header() in django_guid.middleware.process_incoming_request(), which triggers an earlier immutable HttpHeader object build, so any add/override headers intent has no impact.
Would be better to use request.META to get/set headers.

To Reproduce

  1. Create a middleware

    def foo_middleware(get_response):
        def middleware(request):
            request.META["HTTP_X_FOO"] = "foo"
            return get_response(request)
    
        return middleware
  2. Add middleware after django_guid

    MIDDLEWARE = [
        "django_guid.middleware.guid_middleware",
        "foo.foo_middleware",
    ]
  3. Try to access the header in a view

    @api_view(["GET"])
    def test_view(request):
        assert request.headers.get("X-Foo") == "foo"
  4. Now retry by removing "django_guid.middleware.guid_middleware" from MIDDLEWARE.

@alessiofachechi alessiofachechi added the bug Something isn't working label Apr 18, 2023
@JonasKs
Copy link
Member

JonasKs commented Apr 18, 2023

Hi, please see discussion in #95 and the PR #97. I’m out of the country, so hopefully this resolves your issues.

@alessiofachechi
Copy link
Author

alessiofachechi commented Apr 18, 2023

[...] hopefully this resolves your issues.

Unfortunately not @JonasKs.

def get_id_from_header(request: 'HttpRequest') -> str:
    # ...
    header: str = request.headers.get(settings.guid_header_name) 

should theoretically become something like

def get_id_from_header(request: 'HttpRequest') -> str:
    # ...
    header: str = request.META.get(settings.guid_header_name) 

to avoid next middlewares potentially break.

@JonasKs
Copy link
Member

JonasKs commented Apr 18, 2023

Would you mind linking me docs for this? I don't use Django anymore, and I can't remember this behavior.

@JonasKs
Copy link
Member

JonasKs commented Apr 19, 2023

Anyway, I'm very happy to accept a fix with a test for this if you'd like to contribute. I will probably, unfortunately not be able to review for at least a week.

@hartungstenio
Copy link
Contributor

Django describes headers as "a simpler way to access all HTTP-prefixed headers". Since META contains the WSGI variables (which happens to include the headers) , it feels headers is the right way to go.

Overriding request headers in a middleware seems odd. That's why they are immutable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants