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

TypeError: Expected unicode or bytes, got <read-only buffer for 0x10fb7e120, size 5242880, offset 0 at 0x10fb79eb0> #285

Closed
basictheprogram opened this issue Mar 17, 2014 · 20 comments

Comments

@basictheprogram
Copy link

Trying to get bzr push sftp:// working.

bzr with remote repositories requires paramiko and pycrypto.

pycrypto won't compile on clang-5.1 without a patch.

The bzr bug report is here
https://bugs.launchpad.net/bzr/+bug/1293257

The pycrypto bug report is here
https://bugs.launchpad.net/bzr/+bug/1293257

The exception is thrown from paramiko

lib/python2.7/site-packages/paramiko/py3compat.py", line 43, in b
    raise TypeError("Expected unicode or bytes, got %r" % s)
TypeError: Expected unicode or bytes, got <read-only buffer for 0x10fb7e120, size 5242880, offset 0 at 0x10fb79eb0>

But the only thing that changed with the patch to pycrypto to get it to build on clang-5.1. Just attempting to get all developers of all the components on the same page.

@bitprophet
Copy link
Member

N.B. full traceback was posted in the above links (is required to figure out what's going on...)

Replicating the useful parts here:

  File "/Users/tanner/projects/bzr.dev/bzrlib/osutils.py", line 743, in pump_string_file
    write(segment)
  File "/Users/tanner/projects/clients/linvill.com/scripts/lib/python2.7/site-packages/paramiko/file.py", line 313, in write
    data = b(data)
  File "/Users/tanner/projects/clients/linvill.com/scripts/lib/python2.7/site-packages/paramiko/py3compat.py", line 43, in b
    raise TypeError("Expected unicode or bytes, got %r" % s)
TypeError: Expected unicode or bytes, got <read-only buffer for 0x10fb7e120, size 5242880, offset 0 at 0x10fb79eb0>

The relevant part of bzrlib/osutils.py seems to be http://bazaar.launchpad.net/~bzr-pqm/bzr/bzr.dev/view/head:/bzrlib/osutils.py#L739 where it's creating a buffer() object (which seems pretty old, it's in the deprecated section of the 2.6 docs and is not in the Python 3 docs at all).

The processing via b(data) was a straight up addition in the Python 3 compat work, so hopefully all that needs doing is telling b that buffer objects are OK and to be passed through without modifications (and that only under Python 2 since we can't even really see/use them under Python 3 if the above is correct.)

@bitprophet
Copy link
Member

All that aside, @basictheprogram you should be able to skip the Paramiko side of this bug by downgrading your local Paramiko to 1.12 or older - only 1.13 adds this breaking change. If you don't care about Python 3 support (which seems likely given the other codebases' reliance on buffer) 1.13 doesn't have a lot you'd be missing.

To get this fixed for 1.13+, I'd need a reproduction method - I don't run any bzr repositories so I'd need a quick walkthrough of how to reproduce this. (Ideally it would be in a test suite somewhere and I could then just run that test...?)

@andrewsomething
Copy link

@bitprophet You can reproduce this with bzr's testsuite:

bzr selftest --no-plugins bzrlib.tests.commands.test_commit.TestCommitWithBoundBranch.test_commit_mine_modified

On a Debian based system, you'd need to install bzr and python-bzrlib.tests

@jbouse
Copy link

jbouse commented Jul 31, 2014

It looks like PR #352 by jelmer has been submitted as a solution have we validated it fixes it?

@jbouse
Copy link

jbouse commented Aug 25, 2014

Has anyone else tested the #352 PR as an appropriate fix for this issue? I can't test the Debian package as the bzr package maintainer has seen fit to hard code a Conflict against the paramiko package so being able to install both packages and run the test suite is being made harder for me to validate.

@andrewsomething
Copy link

I can confirm it the bzr test no longer fails using jelmer's branch. I ran:

PYTHONPATH=paramiko/build/lib.linux-x86_64-2.7/ bzr selftest --no-plugins bzrlib.tests.commands.test_commit.TestCommitWithBoundBranch.test_commit_mine_modified

@hlieberman
Copy link

This is currently causing the ansible package to conflict with bzr on Debian, and will potentially cause the removal of python-paramiko (and maybe bzr) from Debian, along with everything that depends on it. With the freeze for Jessie coming up soon, the window is rapidly narrowing for this issue to get fixed in time.

@jelmer
Copy link
Contributor

jelmer commented Aug 25, 2014

On Sun, Aug 24, 2014 at 05:27:55PM -0700, Jeremy T. Bouse wrote:

Has anyone else tested the #352 PR as an appropriate fix for this issue? I can't test the Debian package as the bzr package maintainer has seen fit to hard code a Conflict against the paramiko package so being able to install both packages and run the test suite is being made harder for me to validate.

You should be able to reproduce the error and the fix using the bzr
source package
(https://launchpad.net/bzr/2.6/2.6.0/+download/bzr-2.6.0.tar.gz)

and then running the tests from the unpacked tarball

./bzr selftest --no-plugins

(Or by just building the bzr package with the Conflicts dropped)

Cheers,

Jelmer

@jbouse
Copy link

jbouse commented Aug 25, 2014

@hlieberman Even if I (the python-paramiko maintainer) get this patch included and uploaded in a fixed version bzr will still conflict because the bzr maintainer team decided to put a "Conflicts: python-paramiko (>> 1.12.0)" into the debian/control which doesn't help matters just exacerbates the problem. I'm sitting here in the DC14 Hacklab working on packaging issues and have discussed that move with several other DDs who think it was a poor move on the part of the bzr team.

@andrewsomething
Copy link

@jbouse I totally understand why you're unhappy with the situation, adding a Conflicts certainly wasn't anyone's prefered solution. It was done to prevent bzr's removal from testing after it became clear that jelmer's attempts to get this fix in would not be timely enough.

I'm also at DebConf (I'm asb@d.o) and I'm an uploader of the bzr package. I'd be quite happy to coordinate an upload of bzr that removes the Conflicts. Anyways... Let's move this off the upstream issue tracker. I'd be happy to discuss this with you over a drink of some kind here in Portland.

@jelmer
Copy link
Contributor

jelmer commented Aug 25, 2014

(CC: debian bugs)

On Mon, Aug 25, 2014 at 08:50:17AM -0700, Jeremy T. Bouse wrote:

@hlieberman Even if I (the python-paramiko maintainer) get this patch included and uploaded in a fixed version bzr will still conflict because the bzr maintainer team decided to put a "Conflicts: python-paramiko (>> 1.12.0)" into the debian/control which doesn't help matters just exacerbates the problem. I'm sitting here in the DC14 Hacklab working on packaging issues and have discussed that move with several other DDs who think it was a poor move on the part of the bzr team.

This is a regression in paramiko, and this fix is a trivial and
non-risky change. Why can't this bug be fixed in the paramiko package
directly for the time being, why does it have to wait to be fixed in
upstream? I submitted a patch for this in June.

That would have allowed the bug to be fixed in a timely manner
without, preventing the removal of bzr and a lot of other packages
from testing without any intervention from us:

% apt-cache rdepends bzr python-bzrlib | grep -v "|" | uniq | wc -l
49

A Conflict was the quickest way in which I could unbreak bzr for its
users considering the paramiko Debian bug was stalled. I agree this is
not an ideal workaround - but I was hoping paramiko would soon be
fixed, after which I could remove the conflict.

An alternative workaround for me is to patch bzr to disable its paramiko
support if it finds a broken version of paramiko. If this paramiko
bug stays open for much longer then I'll see if I can do that instead.

If there's anything further I can do to help get this bug fixed in paramiko
(upstream or in Debian), please let me know.

Jelmer

Jelmer Vernooij jelmer@samba.org - https://jelmer.uk/

@jelmer
Copy link
Contributor

jelmer commented Aug 25, 2014

On Mon, Aug 25, 2014 at 09:06:16AM -0700, Andrew SB wrote:

I'm also at DebConf (I'm asb@d.o) and I'm an uploader of the bzr package. I'd be quite happy to coordinate an upload of bzr that removes the Conflicts. Anyways... Let's move this off the upstream issue tracker. I'd be happy to discuss this with you over a drink of some kind here in Portland.

Thanks Andrew, I think this bug could do with some higher-bandwidth
communication. :-)

Jelmer Vernooij jelmer@samba.org - https://jelmer.uk/

@bitprophet bitprophet added this to the 1.12.5 et al milestone Aug 25, 2014
@bitprophet
Copy link
Member

Hi, maintainer here. Last time I looked at this the severity re: packaging wasn't nearly as clear - that's super unfortunate :( I'll do what I can to get a bugfix release out today or tomorrow. Thanks for all the input!

@bitprophet
Copy link
Member

Confirmed replication of error as follows (on a Mac OS X 10.8 system):

# Setup
brew install bzr
cd ~/tmp
# As per instructions on bzr's website
bzr init-repo bzr
cd bzr
bzr branch lp:bzr bzr-dev
cd bzr-dev
# Get this bzr installed as my local bzr
brew unlink bzr
mkvirtualenv bzrstuff
pip install pyrex # took a few minutes to muddle this one out :)
pip install -e .
# Run given command, see no error, get confused
bzr selftest --no-plugins bzrlib.tests.commands.test_commit.TestCommitWithBoundBranch.test_commit_mine_modified
# Override the (vendored? found no references to 'paramiko' in my virtualenv after the above)
# paramiko with a local copy of master
pip install -e ~/Code/oss/paramiko
# Now this gives the buffer error
bzr selftest --no-plugins bzrlib.tests.commands.test_commit.TestCommitWithBoundBranch.test_commit_mine_modified

Now testing out the linked fix...

@bitprophet
Copy link
Member

Yup, works fine. Regular test suite still passes too. Changelogging & merging, will close this post-feedback from the affected users.

bitprophet added a commit that referenced this issue Aug 26, 2014
@bitprophet
Copy link
Member

Merged and pushed to master. Backporting to release branches momentarily.

bitprophet pushed a commit that referenced this issue Aug 26, 2014
…ted.

This fixes bzr's use of paramiko.

Fixes issue #343/#285.
bitprophet added a commit that referenced this issue Aug 26, 2014
Conflicts:
	sites/www/changelog.rst
@bitprophet
Copy link
Member

This is now in the 1.13 and 1.14 lines in addition to master. I'll be pushing out a release tomorrow sometime, probably the early half of the day Pacific time. Again, feedback before then is welcome.

@bitprophet
Copy link
Member

1.13.2 and 1.14.1 are on PyPI now. (This isn't what I meant by "the early half of the day tomorrow", but hey, if the shoe fits.)

@jbouse
Copy link

jbouse commented Aug 26, 2014

Thank you @bitprophet. I've found the 1.14.1 release tarball. I'm just having some local package maintenance repo issues of my own fault but I should have the Debian package produced and uploaded soon which should make @jelmer very happy.

@bitprophet
Copy link
Member

Glad to hear it @jbouse, please keep me posted, and thanks :)

dkhapun pushed a commit to cyberx-labs/paramiko that referenced this issue Jun 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants