Concurrent writes to a remote-signing catalog go out unsigned and fail 403
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 1/5
- Thời gian dự kiến
- Dưới một giờ
- Mức phù hợp với người mới
- 35/100
Hướng nghiên cứu
Bắt đầu trong pyiceberg/io/fsspec.py tại _s3(), sau đó chạy hai test được nêu tên trong tests/io/test_fsspec.py. Được coi là hoàn tất khi signer vẫn được cài đặt trong lúc reconfiguration của shared-client diễn ra và cả hai concurrency regression test đều vượt qua; PR #3783 đã bao gồm bản sửa lỗi được báo cáo.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
Apache Iceberg version
main (development) — also reproduced on 0.12.0 and 0.11.1.
Please describe the bug 🐞
_s3() unregisters the S3 request signer and re-registers it on an event emitter that fsspec caches and every thread shares. A request signed in that window goes out with no Authorization header — _s3() has already set config_kwargs["signature_version"] = UNSIGNED, so botocore does not sign in its place — and the store answers 403 AccessDenied.
pyiceberg/io/fsspec.py, in _s3():
fs = S3FileSystem(**s3_fs_kwargs)
for event_name, event_function in register_events.items():
fs.s3.meta.events.unregister(event_name, unique_id=1925) # opens the window
fs.s3.meta.events.register_last(event_name, event_function, unique_id=1925)
FsspecFileIO.get_fs caches per thread and every table gets its own FileIO, so a short workload makes hundreds of these cycles against the one shared emitter. PyIceberg's writer is concurrent by default, so no unusual usage is needed to reach it.
Roughly 3–4% of appends fail against a remote-signing catalog, surfacing as an opaque PermissionError: Access Denied out of s3fs, several frames from its cause. Every unsigned request caught on the wire was a PUT of a manifest during commit.
Measured on Lakekeeper 0.13.1 + MinIO (path-style), 90 writes per run:
| configuration | append failures | unsigned on wire |
|---|---|---|
| as shipped | 2 / 4 / 5 | 2 / 4 / 5 |
| signer registered once (client still shared) | 0 | 0 |
PYICEBERG_MAX_WORKERS=1 |
0 | 0 |
The second row is the one that matters: the failures stop while the client is still shared, which separates this from a general concurrency problem. The response code is always AccessDenied and never SignatureDoesNotMatch — what an unsigned request produces, not a mis-signed one.
Suggested fix
Drop the unregister. Botocore's HierarchicalEmitter._register_section returns early for a unique_id it already holds, so re-registering an equivalent signer was already a no-op; the unregister only opens the window.
Reproduction
Two tests in tests/io/test_fsspec.py, no credentials and no network:
test_s3_leaves_a_signer_installed_while_reconfiguring_a_shared_client— drives_s3()and observes the emitter the instant it unregisters.test_the_signer_stays_installed_while_another_thread_reconfigures_s3— the same window seen from another thread. The window is two adjacent statements, so sampling for it blind is a coin flip (20,000 observations caught it zero times); it is held open by delaying only the re-registration, so the timing is deterministic while the defect is not manufactured.
Both fail on main and pass with the fix.
Already covered by an open PR
#3783 (for #3625) removes this unregister as part of registering the signer on an AioSession, for a different reason — a lazily created client not inheriting handlers, failing InvalidRequest. I applied that PR and ran both tests above against it: both pass, so it fixes this as well.
Worth recording rather than closing silently, because the line is wrong for two independent reasons and #3783's own test does not cover this one. If #3783 lands, these two tests are the regression cover for the concurrency window; happy to raise them against that PR instead if a maintainer prefers.
A second, independent defect in the same file — noted, not proposed here
pyiceberg/io/fsspec.py:160 applies the signing service's headers with add_header, which appends, so every header the service echoes back appears twice — 880 of 1615 sign calls ended with a duplicate of a header named in SignedHeaders. On MinIO this is latent: de-duplicating changed nothing, and it did not contribute to the 403s above. SigV4 combines repeated headers comma-separated, so a stricter store may reject them. Mentioned so it is not lost, not to widen this issue.
Willingness to contribute
I can contribute a fix to resolve this bug independently.
- Ngôn ngữ chính
- Python
- Star
- 1.1k
- Fork
- 589
- Merge trung bình
- 1 ngày 20 giờ
- Pull request đã merge (30 ngày)
- 68
Hướng dẫn đóng góp
Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của apache/iceberg-python
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 70/100
apache/iceberg-python#4010 · 1 reaction ·
-
kind:bug
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 92/100
apache/iceberg-python#4006 ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
apache/iceberg-python#3996 ·
-
Deletion vector bitmap count is read from the blob and used as a loop bound without validation Đang mởbug
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 72/100
apache/iceberg-python#3979 ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
apache/iceberg-python#3885 ·
Tất cả issue của apache/iceberg-python
Issue tương tự
-
bug
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 75/100
xinnan-tech/xiaozhi-fde-talk#263 ·
-
rules
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 90/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 70/100
huggingface/Repo2RLEnv#163 · 1 bình luận ·
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 95/100
huggingface/sentence-transformers#4074 ·
-
comp/dashboard invalid P3
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 70/100
NousResearch/hermes-agent#121143 ·