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

RequestMiddleware forces vary: Cookie on each request #734

Open
last-partizan opened this issue Jan 7, 2025 · 7 comments
Open

RequestMiddleware forces vary: Cookie on each request #734

last-partizan opened this issue Jan 7, 2025 · 7 comments

Comments

@last-partizan
Copy link

Hello.

Today I was trying to understand why my app have vary: Cookie on each request, even when I do not use request.user in my view.

Turned out, RequestMiddleware uses request.user, which in turn uses request.session which in turn sets request.session.accessed to True, and then SessionMiddleware adds patch_vary_headers(response, ("Cookie",)).

I fixed this by putting RequestMiddleware before SessionMiddleware, but this removes user information from context.

Maybe user_id can be made lazy, so it won't trigger access to request.user unless it really needed?

If this a desirable change, I can prepare PR.

Maybe you have ideas how to best implement this, or some alternative suggestions?

@jrobichaud
Copy link
Owner

Please can you reproduce the issue with a failing test first. I'll then take a look at it.

@last-partizan
Copy link
Author

@jrobichaud Sure, do you have any tests for checking how middleware works? or where i should put it?

@jrobichaud
Copy link
Owner

See doc on how to run tests locally:
https://django-structlog.readthedocs.io/en/latest/running_tests.html

There is a bunch of tests starting here:

def test_process_request_without_user(self) -> None:

@last-partizan
Copy link
Author

Yeah, found it already.

But, we need to test not only RequestMiddleware, but entire django stack and how it works with SessionMiddleware, so I created separate file.

New test fails with:

        assert response.status_code == 200
>       assert response.headers.get("Vary") is None
E       assert 'Cookie' is None
E        +  where 'Cookie' = get('Vary')

@jrobichaud
Copy link
Owner

Thanks, I know how to reproduce it now.

I'll look at this by the end of next week.

@last-partizan
Copy link
Author

Oh, now I see what that middleware doing.

2025-01-07T13:17:43.447584Z [info     ] request_started                [django_structlog.middlewares.request] ip=127.0.0.1 request=GET /noop request_id=1f66f3ef-19a5-4a03-b942-2c3dcb10829a user_agent=None user_id=None
2025-01-07T13:17:43.447900Z [info     ] request_finished               [django_structlog.middlewares.request] code=200 ip=127.0.0.1 request=GET /noop request_id=1f66f3ef-19a5-4a03-b942-2c3dcb10829a user_id=None

It logs the data... And even if we replace user_id with lazy variable, this logging will trigger access to request.user.

And I was under impression that it only adds context variables to be used in other log calls. Not sure how to proceed in this case.

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

2 participants