Skip to content

fix(streaming): _S3SeekableIO.__next__/__iter__ don't advance self._position, corrupting subsequent seek()/read() - #8389

Merged
leandrodamascena merged 3 commits into
aws-powertools:developfrom
Adityaj0:fix/s3-seekable-io-iteration-position-tracking
Sep 14, 2026
Merged

leandrodamascena merged 3 commits into
aws-powertools:developfrom
Adityaj0:fix/s3-seekable-io-iteration-position-tracking

Conversation

@Adityaj0

@Adityaj0 Adityaj0 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Issue number

Fixes #8388

Summary

Fixes a bug where iterating an _S3SeekableIO/S3Object stream (for line in s3_object: ... or next(s3_object)) leaves self._position stuck at its pre-iteration value, so a subsequent seek() computes the wrong S3 byte range and the following read() silently returns the wrong slice of the object.

Changes

__next__ and __iter__ previously delegated straight to raw_stream.__next__()/raw_stream.__iter__():

def __next__(self):
    return self.raw_stream.__next__()

def __iter__(self):
    return self.raw_stream.__iter__()

Unlike read()/readline()/readlines() (which all advance self._position by the number of bytes actually consumed), this bypassed position bookkeeping entirely. seek() uses self._position as ground truth to build the Range header on the next raw_stream access, so any seek() after iterating would compute an offset relative to a stale position.

Fix: route __next__ through the already position-tracked readline(), and have __iter__ return self, matching the standard Python file-iterator protocol:

def __next__(self):
    line = self.readline()
    if not line:
        raise StopIteration
    return line

def __iter__(self):
    return self

This is functionally equivalent for consumers that only care about line-by-line iteration (the common case), while keeping position tracking correct for anyone who seeks after iterating.

User experience

Before this fix, code like:

for line in s3_object:
    ...
s3_object.seek(5, io.SEEK_CUR)
s3_object.read(...)

would silently read the wrong bytes after the seek, with no exception raised. After this fix, position tracking stays correct across iteration, so seek()/read() behave correctly regardless of whether the stream was previously consumed via .read() or via iteration.

Checklist

  • My changes are covered by tests
  • I have added/updated the develop branch docs, if applicable — N/A, internal bug fix, no public API/behavior change for correctly-functioning code paths

Is this a breaking change?

NO. __iter__ returning self instead of raw_stream's iterator, and __next__ routing through readline(), are both drop-in-compatible with the existing documented iteration usage (for line in s3_object, next(s3_object)) — only the previously-broken position tracking changes.

Tests

Added to tests/functional/streaming/_boto3/test_s3_seekable_io.py:

  • test_next_advances_position — verifies tell() correctly reflects bytes consumed via next()
  • test_iter_returns_self — standard iterator protocol check
  • test_seek_after_iteration_uses_correct_range — reproduces the real failure mode: iterate the stream, then seek, and verify the resulting Range header reflects true consumed position rather than the stale pre-iteration value

All three fail against the pre-fix code (verified by reverting the fix locally and re-running; the seek test shows the exact bug live: expected bytes=28-, received bytes=5-) and pass after the fix.

Full suite tests/functional/streaming/_boto3/test_s3_seekable_io.py: 24 passed (independently re-verified).


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…osition, corrupting subsequent seek()/read()

__next__ and __iter__ delegated straight to raw_stream.__next__()/raw_stream.__iter__(),
bypassing self._position bookkeeping entirely -- unlike read()/readline()/readlines(),
which all correctly advance self._position by the number of bytes actually consumed.

seek() uses self._position as ground truth to compute the S3 GetObject Range header on
the next raw_stream access. Consuming any data via the iterator protocol (e.g.
"for line in s3_object: ...", a fully supported documented usage) left self._position
stuck at its pre-iteration value. A subsequent seek()/read() would then reopen the S3
stream with a Range header computed from that stale position -- silently returning the
wrong slice of the object (skipped or duplicated bytes), with no exception raised.

Fix: route __next__ through the already position-tracked readline(), and have __iter__
return self, matching the standard Python file-iterator protocol.
@Adityaj0
Adityaj0 requested a review from a team as a code owner August 15, 2026 10:10
@Adityaj0
Adityaj0 requested a review from hjgraca August 15, 2026 10:10
@powertools-for-aws-oss-automation powertools-for-aws-oss-automation Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 15, 2026
@sonarqubecloud

Copy link
Copy Markdown

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching the position tracking issue. The bug is real, but I think the fix needs one adjustment before we merge it.

StreamingBody iteration returns 1 KiB chunks, not lines. Routing __next__() through readline() changes the existing behavior. I reproduced this with the same payload:

  • current behavior: [1024, 481]
  • this PR: [2, 1501, 2]

For an object without newlines, next() can also read the entire remaining object instead of a 1 KiB chunk.

Could we preserve the current iteration contract and only add the missing position tracking?

def __next__(self):
    chunk = next(self.raw_stream)
    self._position += len(chunk)
    return chunk

def __iter__(self):
    return self

The tests should also use a payload larger than 1 KiB, confirm the chunk size remains unchanged, and test a partial iteration followed by SEEK_CUR and an actual read(). The current seek test consumes the whole object and then seeks beyond EOF, so it verifies the generated range but not the returned data.

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.65%. Comparing base (e0566b0) to head (9342bf9).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8389      +/-   ##
===========================================
+ Coverage    96.62%   96.65%   +0.03%     
===========================================
  Files          296      296              
  Lines        14883    14885       +2     
  Branches      1263     1263              
===========================================
+ Hits         14380    14387       +7     
+ Misses         366      363       -3     
+ Partials       137      135       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sonarqubecloud

Copy link
Copy Markdown

@leandrodamascena
leandrodamascena merged commit 78ce2cb into aws-powertools:develop Sep 14, 2026
15 checks passed
@mergify

mergify Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

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

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files. streaming tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

S3Object streaming: iterating a stream corrupts subsequent seek()/read() by silently returning the wrong bytes

2 participants