airflow icon indicating copy to clipboard operation
airflow copied to clipboard

Fix XCom key handling when keys contain special characters like slash

Open henry3260 opened this issue 1 month ago • 3 comments

This PR picks up the work from @Brunda10 in PR #56042, which had become stale.

  • Fixes 404 errors for XCom keys containing special characters (e.g., /).
  • Keys are now URL-encoded (quoted) before being sent to the API.
  • Keys are decoded (unquoted) when received from the API.
  • Adds required unit tests to verify the fix, as requested in the original PR.

Closes: #55410


^ Add meaningful description above Read the Pull Request Guidelines for more information. In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed. In case of a new dependency, check compliance with the ASF 3rd Party License Policy. In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

henry3260 avatar Nov 15 '25 13:11 henry3260

Nice!

It would be nice to add pytest.mark.parametrize with slash value key in task-sdk-integration-tests/tests/task_sdk_tests/test_xcom_operations.py tests.

Thanks!

Thanks for reviewing!

Slash keys aren’t supported today: the route pattern /{dag_id}/{run_id}/{task_id}/{key} treats {key} as a single path segment, so "folder/sub/value" splits into multiple segments and won’t match. Adding support means restructuring the routes (e.g. {key:path} plus reworking /item/{offset} and /slice), or switching to a query param (?key=). That’s a broader change, so I propose we keep the simple key test in this PR, optionally mark a slash-key test as xfail, and follow up with a dedicated issue/PR for proper support. If you prefer to fold the refactor into this PR, let me know—just want to confirm scope first.

henry3260 avatar Nov 22 '25 10:11 henry3260

@henry3260 Do you mind rebasing to fix the conflicts. We can probably go ahead and merge, the PR is looking good.

Sure

henry3260 avatar Dec 04 '25 14:12 henry3260

@henry3260 Do you mind rebasing to fix the conflicts. We can probably go ahead and merge, the PR is looking good.

All CI tests have passed. Ready for merge!

henry3260 avatar Dec 05 '25 05:12 henry3260

Thanks for the PR.

pierrejeambrun avatar Dec 11 '25 11:12 pierrejeambrun

Backport successfully created: v3-1-test

Status Branch Result
v3-1-test PR Link

github-actions[bot] avatar Dec 11 '25 11:12 github-actions[bot]