Skip to content

Unbounded growth in SpooledTemporaryFile.writelines() #127371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bertptrs opened this issue Nov 28, 2024 · 5 comments
Closed

Unbounded growth in SpooledTemporaryFile.writelines() #127371

bertptrs opened this issue Nov 28, 2024 · 5 comments
Assignees
Labels
type-bug An unexpected behavior, bug, or error

Comments

@bertptrs
Copy link
Contributor

bertptrs commented Nov 28, 2024

Bug report

Bug description:

SpooledTemporaryFile provides a temporary file backed by a buffer in memory that spills over to disk when it gets too large. However, the writelines() method only checks whether it should roll over after the entire lines iterator is exhausted. This causes unexpectedly high memory use when feeding it large iterators.

from tempfile import SpooledTemporaryFile

with SpooledTemporaryFile(mode="w", max_size=1024) as temp:
    temp.writelines(map(str, range(1000)))

With the above code, one might expect that the buffer doesn't grow (much) past 1024 bytes, but it grows to almost three times that size before finally rolling over.

CPython versions tested on:

3.12, 3.13, CPython main branch

Operating systems tested on:

Linux

Linked PRs

@serhiy-storchaka
Copy link
Member

Thank you for your report and PR @bertptrs. I do not think that it is worth to backport this change, it looks more as a new feature (minor improvement of memory use).

@bertptrs
Copy link
Contributor Author

bertptrs commented Mar 5, 2025

@serhiy-storchaka I personally do think is better to backport, as it is a security issue. I discovered this bug because of a DoS issue in our code base. The developer who wrote the original code reasonably assumed that he had set a buffer size limit, while he effectively hadn't.

How much effort is it to backport this?

@serhiy-storchaka
Copy link
Member

I'm not strictly against it, I just have doubts. I left this on the release manager, @Yhg1s.

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 5, 2025
@Yhg1s
Copy link
Member

Yhg1s commented Mar 5, 2025

Backporting the fix makes sense to me.

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 5, 2025
Yhg1s pushed a commit that referenced this issue Mar 5, 2025
…H-127372) (#130885)

gh-127371 Avoid unbounded growth SpooledTempfile.writelines (GH-127372)
(cherry picked from commit cb67b44)

Co-authored-by: Bert Peters <[email protected]>
Yhg1s pushed a commit that referenced this issue Mar 5, 2025
…H-127372) (#130886)

gh-127371 Avoid unbounded growth SpooledTempfile.writelines (GH-127372)
(cherry picked from commit cb67b44)

Co-authored-by: Bert Peters <[email protected]>
@bertptrs
Copy link
Contributor Author

bertptrs commented Mar 6, 2025

Thanks for backporting! Looking forward to seeing these changes drop in a release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants