You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Sriniketh24 (via GitHub)" <gi...@apache.org> on 2026/05/18 23:33:48 UTC

[PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Sriniketh24 opened a new pull request, #67141:
URL: https://github.com/apache/airflow/pull/67141

   ## Summary
   
   `_LazySafeSerializer.dumps()` uses `msgspec.msgpack` which returns `bytes`, but Werkzeug 3.0+ removed the automatic `bytes → str` coercion in `dump_cookie()`. This causes a `TypeError` when the session cookie is set, breaking login for FAB users on Werkzeug 3.0+.
   
   **Fix:** Switch the serializer from `msgspec.msgpack` to `msgspec.json` so `dumps()` returns `str`. The `loads()` method tries JSON first and falls back to msgpack for backward compatibility with sessions written before this change.
   
   closes: #64921
   
   ## Changes
   
   - **`providers/fab/src/airflow/providers/fab/www/session.py`**: Switch `_LazySafeSerializer` from `msgspec.msgpack` (bytes) to `msgspec.json` (str) encoding. `loads()` now tries JSON first and falls back to msgpack for legacy session data.
   - **`providers/fab/tests/unit/fab/www/test_session.py`**: Add 8 tests covering str return type, roundtrip, LazyString conversion, legacy msgpack backward compatibility, empty session, nested data, encode/decode aliases, and str/bytes input handling.
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Claude Code (Opus 4.6)
   
   Generated-by: Claude Code (Opus 4.6) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #67141:
URL: https://github.com/apache/airflow/pull/67141#issuecomment-4483645851

   @Sriniketh24 Converting to **draft** — this PR doesn't yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
   
   - :x: **Provider tests** — Failing: `Low dep tests: providers / All-prov:LowestDeps:14:3.10:common.compat...fab`, `provider distributions tests / Compat 3.0.6:P3.10:`, `provider distributions tests / Compat 3.1.8:P3.10:`, `provider distributions tests / Compat 3.2.1:P3.10:`. See [docs](https://github.com/apache/airflow/blob/main/contributing-docs/12_provider_distributions.rst).
   
   
   See the linked criteria for how to fix each item, then mark the PR "Ready for review". This is **not** a rejection — just an invitation to bring the PR up to standard. No rush.
   
   ---
   
   _Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
   
   ---
   
   Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "Sriniketh24 (via GitHub)" <gi...@apache.org>.
Sriniketh24 commented on PR #67141:
URL: https://github.com/apache/airflow/pull/67141#issuecomment-4531469912

   Thanks for the heads-up! I've pushed a fix for the CI failures — the test mocks had two issues:
   
   1. **Wrong MRO target**: `patch.object` was patching `__mro__[1]` (SessionExemptMixin) instead of `__mro__[2]` (SecureCookieSessionInterface). This meant `SessionExemptMixin.save_session()` — which accesses `flask.request.path` — was bypassed entirely, and the bytes-to-str wrapper in `AirflowSecureCookieSessionInterface.save_session()` wasn't being exercised through the real call chain.
   
   2. **Lambda signature mismatch**: `patch.object` replaces the class method with a `MagicMock` whose `side_effect` does **not** receive `self` when called via `super()`. The lambda expected `(self, app, session, response)` but only received `(app, session, response)`.
   
   The fix patches `__mro__[2]` directly, adds `patch("flask.request")` so SessionExemptMixin can check `request.path`, and uses `lambda *args, **kwargs:` for the side_effect.
   
   ---
   Drafted-by: Claude Code (Opus 4.7) (no human review before posting)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "Sriniketh24 (via GitHub)" <gi...@apache.org>.
Sriniketh24 commented on PR #67141:
URL: https://github.com/apache/airflow/pull/67141#issuecomment-4526448792

   Reworked the approach based on the CI failures:
   
   **Previous approach** (broken): Changed `_LazySafeSerializer` from msgpack to JSON encoding. This caused `TypeError: memoryview: a bytes-like object is required, not 'str'` in compat tests because JSON bytes (valid UTF-8) were being coerced to `str` somewhere in the flask-session → SQLAlchemy pipeline, breaking the `LargeBinary` column insert.
   
   **New approach**: Leave the database serializer (`_LazySafeSerializer`) unchanged — msgpack is correct for `LargeBinary` columns. Instead, fix the actual bug site: override `save_session()` in `AirflowSecureCookieSessionInterface` to wrap `response.set_cookie()` with bytes-to-str coercion, since Werkzeug 3.0+ [removed bytes support in `dump_cookie()`](https://github.com/pallets/werkzeug/commit/5ff0a573f4b78d9724f1f063fb058fd6bc76b24d).
   
   ---
   Drafted-by: Claude Code (Opus 4.6); reviewed by @Sriniketh24 before posting


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "Sriniketh24 (via GitHub)" <gi...@apache.org>.
Sriniketh24 commented on PR #67141:
URL: https://github.com/apache/airflow/pull/67141#issuecomment-4540745820

   Pushed another fix — identified the actual root cause from the CI logs.
   
   **Root cause:** `session.py` binds `request` in its own namespace via `from flask import request` at import time. Patching `flask.request` has no effect on that already-evaluated reference — `SessionExemptMixin.save_session` reads from `airflow.providers.fab.www.session.request` (the local binding), not from `flask.request`.
   
   **Fix:** Changed both mock targets from `patch("flask.request")` to `patch("airflow.providers.fab.www.session.request")`. The mock now intercepts the exact name the code uses, so `request.path == "/any"` works correctly and no `RuntimeError: Working outside of request context` is raised.
   
   All other logic (MRO[2] target, `lambda *args, **kwargs`) remains correct. The fix is purely this one-line patch-target correction (plus an explanatory comment).
   
   Would you be able to move this back out of draft once CI passes? Happy to address any other feedback.
   
   ---
   Drafted-by: Claude Code (claude-sonnet-4-6) (no human review before posting)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #67141:
URL: https://github.com/apache/airflow/pull/67141#issuecomment-4538558019

   @Sriniketh24 Converting to **draft** — this PR doesn't yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
   
   - :x: **Provider tests**. See [docs](https://github.com/apache/airflow/blob/main/contributing-docs/12_provider_distributions.rst).
   
   See the linked criteria for how to fix each item, then mark the PR "Ready for review". This is **not** a rejection — just an invitation to bring the PR up to standard. No rush.
   
   ---
   
   _Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
   
   ---
   Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "ashwani4588 (via GitHub)" <gi...@apache.org>.
ashwani4588 commented on PR #67141:
URL: https://github.com/apache/airflow/pull/67141#issuecomment-4550998881

   This is already fixed and released as part of Airflow providers fab 3.6.4 under pr#66565. Already tested and working fine. Thank you. Earlier with 3.6.1 i had used explicit type conversion in webserver_config.py to handle the error. Thanks a lot guys for working on this issue. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #67141:
URL: https://github.com/apache/airflow/pull/67141#issuecomment-4529640468

   @Sriniketh24 A few things need addressing before review — see our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
   
   - :x: **CI fails**: `Low dep tests: providers / All-prov:LowestDeps:14:3.10:common.compat...fab` (and possibly other checks — see the [Checks tab](https://github.com/apache/airflow/pull/67141/checks) for the full list).
   
   No rush.
   
   ---
   
   _Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
   
   ---
   Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #67141:
URL: https://github.com/apache/airflow/pull/67141#issuecomment-4483138993

   Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our [Contributors' Guide](https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks) will help you with that.
   - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example Dag that shows how users should use it.
   - Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
   - Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
   - Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
   - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
   - Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
   Apache Airflow is a community-driven project and together we are making it better 🚀.
   In case of doubts contact the developers at:
   Mailing List: dev@airflow.apache.org
   Slack: https://s.apache.org/airflow-slack
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "Sriniketh24 (via GitHub)" <gi...@apache.org>.
Sriniketh24 closed pull request #67141: Fix FAB session cookie TypeError with Werkzeug 3.0+
URL: https://github.com/apache/airflow/pull/67141


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix FAB session cookie TypeError with Werkzeug 3.0+ [airflow]

Posted by "Sriniketh24 (via GitHub)" <gi...@apache.org>.
Sriniketh24 commented on PR #67141:
URL: https://github.com/apache/airflow/pull/67141#issuecomment-4551017446

   Closing this in favour of #66565 which was merged 2026-05-08 and addresses the same issue at the root cause level.
   
   #66565 moved `self.serializer = _LazySafeSerializer()` from `SessionExemptMixin.__init__` into `AirflowDatabaseSessionInterface.__init__` only, so `AirflowSecureCookieSessionInterface` now uses Flask's default string-based serializer and the bytes-vs-str mismatch never occurs. That is a cleaner fix than the bytes-to-str coercion in `save_session()` that this PR applied.
   
   Thanks @potiuk for the review and @ashwani4588 for pointing out the existing fix.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org