ghoshp83 opened a new pull request, #3999:
URL: https://github.com/apache/iceberg-python/pull/3999
Addresses the first half of #3970.
# Rationale for this change
`RestCatalog.commit_table` bound the session's own header mapping and then
wrote
the table-scoped token into it:
```python
headers = self._session.headers # the live mapping, not a copy
if table_token := table.config.get(TOKEN):
headers[AUTHORIZATION_HEADER] = f"{BEARER_PREFIX} {table_token}"
```
`self._session` is shared by every table the catalog serves, so the
assignment
outlives the commit. The token stays on the session and is carried by later
requests, including requests for other tables.
`headers` is already passed explicitly to `self._session.post(...)`, so
building
it from a copy sends exactly the same request while leaving the session
alone.
# Are these changes tested?
Yes — `test_commit_table_does_not_leak_table_token_onto_session` in
`tests/catalog/test_rest.py` loads a table whose config carries a token,
commits,
and asserts the session has no `Authorization` header afterwards. On `main`
the
assertion fails, because the session is left holding
`Bearer table_scoped_token`.
`tests/catalog/test_rest.py` passes (166 passed; the one unrelated failure,
`test_rest_catalog_with_google_credentials_path`, is a missing optional
`google`
dependency in my environment and fails identically on `main`).
# Are there any user-facing changes?
No. The request the catalog sends is unchanged; only the lifetime of the
header
changes.
# Left out of this PR
Two related findings, kept separate so this stays one concern:
- Writing the fix surfaced that the table-scoped token never reaches the
wire at
all. `RestCatalog` sets `session.auth = AuthManagerAdapter(...)`, and
`requests` applies `session.auth` after per-request headers are merged, so
the
auth manager overwrites `Authorization`. Both before and after this
change, the
commit goes out with the catalog's token. I have written this up on #3970
rather than change it here, because deciding whether a table-scoped token
should override the auth manager is a design question.
- The `S3FileSystem` signer sharing described in the second half of #3970 is
untouched; it is also commented on the 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
