Skip to content

GCP: Reject invalid GCS buffered read ranges - #17961

Open
charliec05 wants to merge 2 commits into
apache:mainfrom
charliec05:agent/gcp-read-bounds
Open

GCP: Reject invalid GCS buffered read ranges#17961
charliec05 wants to merge 2 commits into
apache:mainfrom
charliec05:agent/gcp-read-bounds

Conversation

@charliec05

Copy link
Copy Markdown
Contributor

Summary

  • validate offset and length in GCSInputStream.read(byte[], int, int)
  • stop silently truncating reads whose requested range exceeds the destination buffer
  • cover overflow, negative offset, and negative length cases

Motivation

GCSInputStream used Math.min when setting the reusable ByteBuffer limit. A request such as reading three bytes at offset two into a four-byte array was silently shortened to two bytes instead of throwing IndexOutOfBoundsException as required by the InputStream contract.

The existing EOF test also passed a length larger than its buffer; it now uses the buffer's actual length so it continues to exercise EOF with a valid range.

Testing

  • ./gradlew :iceberg-gcp:spotlessApply :iceberg-gcp:test

AI Disclosure

  • Model: GPT-5 (Codex)
  • Platform/Tool: OpenAI Codex
  • Human Oversight: fully reviewed
  • Prompt Summary: Used a detailed, repository-aware prompt to inspect buffered read contracts, reproduce silent range truncation, implement explicit validation, review the resulting diff, and run the complete affected module test suite.

Validate buffered read offsets and lengths instead of silently truncating out-of-bounds requests.

Generated-by: Codex
@github-actions github-actions Bot added the GCP label Sep 4, 2026
@charliec05

Copy link
Copy Markdown
Contributor Author

Hi @szehon-ho, would you be willing to review this GCSInputStream contract fix? It rejects invalid buffered-read ranges instead of silently truncating them, includes boundary regression coverage, and all 42 CI checks pass. Thanks!

@uros-b

uros-b commented Sep 5, 2026

Copy link
Copy Markdown
Member

+1, thank you @charliec05! Yes please ping @szehon-ho for additional review here

@charliec05

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @uros-b. @szehon-ho, could you take a look when you have time? The current revision has uros-b's approval and all 42 checks pass; there are no outstanding code-change requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants