Skip to content
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

bpo-39681: Fix C pickle regression with minimal file-like objects #18592

Merged
merged 1 commit into from
Feb 23, 2020

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Feb 21, 2020

Fix a regression where the C pickle module wouldn't allow unpickling from a
file-like object that doesn't expose a readinto() method.

https://bugs.python.org/issue39681

Fix a regression where the C pickle module wouldn't allow unpickling from a
file-like object that doesn't expose a readinto() method.
@pitrou
Copy link
Member Author

pitrou commented Feb 21, 2020

cc @pierreglaser and @ogrisel if you want to give this a quick look.

@codecov
Copy link

codecov bot commented Feb 21, 2020

Codecov Report

Merging #18592 into master will increase coverage by 1.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #18592       +/-   ##
===========================================
+ Coverage   82.11%   83.13%    +1.01%     
===========================================
  Files        1956     1571      -385     
  Lines      589402   414817   -174585     
  Branches    44458    44458               
===========================================
- Hits       484012   344841   -139171     
+ Misses      95739    60348    -35391     
+ Partials     9651     9628       -23     
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
Lib/test/test_asyncio/test_base_events.py 91.84% <0.00%> (-3.30%) ⬇️
... and 436 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4d17fd...7a3d53c. Read the comment docs.

return bad_readline();
}
if (_Unpickler_SkipConsumed(self) < 0) {
return -1;
}

if (!self->readinto) {
/* readinto() not supported on file-like object, fall back to read()
* and copy into destination buffer (bpo-39681) */
Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps we should deprecate this fallback path in 3.9?

@ambv ambv merged commit 9f37872 into python:master Feb 23, 2020
@bedevere-bot
Copy link

@ambv: Please replace # with GH- in the commit message next time. Thanks!

@miss-islington
Copy link
Contributor

Thanks @pitrou for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-18630 is a backport of this pull request to the 3.8 branch.

@pitrou pitrou deleted the bpo-39681-pickle-readinto branch February 23, 2020 22:44
ambv pushed a commit that referenced this pull request Feb 23, 2020
…-18592) (#18630)

Fix a regression where the C pickle module wouldn't allow unpickling from a
file-like object that doesn't expose a readinto() method.
(cherry picked from commit 9f37872)

Co-authored-by: Antoine Pitrou <antoine@python.org>

Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@ogrisel
Copy link
Contributor

ogrisel commented Feb 24, 2020

LGTM. Thanks for the heads up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants