-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
Unicode bug while using fetch module #4014
Comments
Thanks for the heads up. I'd say "exposed" not "introduced" maybe :) On Tue, Sep 3, 2013 at 8:25 PM, Mustafa Khattab notifications@github.comwrote:
|
Exposed sounds better. :) Perhaps it would be better to just pass the variable |
But I guess it would break things if we just passed |
If you add the 'ignore', does it fail the md5 check? That would be my only concern with modifying this. (edited: didn't realize bytearray just used str.encode() under the hood). |
I'll go ahead and try this. I think it would fail since I'm fetching a binary file (gzipped tarball). |
Update: it doesn't work as I expected. TASK: [fetch tar file] ********************************************************
failed: [192.168.33.10] => {"dest": "/Users/mustafa/sandbox/ansibletest/tmp/192.168.33.10/tmp/temp.tar.gz", "failed": true, "file": "/tmp/temp.tar.gz", "md5sum": "6747c85f736cf
bf62df0177c649a1b7f"}
msg: md5 mismatch
FATAL: all hosts have already failed -- aborting
PLAY RECAP ********************************************************************
to retry, use: --limit @/Users/mustafa/test.retry
192.168.33.10 : ok=1 changed=0 unreachable=0 failed=1
|
I'm hitting a wall with this one too, I'm also fetching a gz'd tarball. Trying to understand what the code is doing exactly -- what is it attempting to utf-8 encode exactly? |
It's trying to encode the data being fed to the md5 module to prevent it from failing, according to the original patch. The problem is, the same invalid characters that can trip up the md5 module are tripping up str.encode() it seems. |
This patch seems to fix the issue for me, if you guys would like to test it: diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index b5f1886..de6ab6b 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -393,8 +393,9 @@ def merge_hash(a, b): def md5s(data): ''' Return MD5 hex digest of data. ''' + buf = StringIO.StringIO(data) digest = _md5() - digest.update(data.encode('utf-8')) + digest.update(buf.read()) return digest.hexdigest() def md5(filename): |
Thanks @jimi1283 -- patch works for me. |
This breaks ansible for me:
|
First and foremost, here's the stack trace (running off
devel
@2696135b):I think the bug is introduced by commit faf82bf, after which
digest.update(data.encode('utf-8'))
L397 is called and the error is thrown.I'm not sure if doing
data.encode('utf-8', 'ignore')
would change the md5 sum. That might introduce other issues.The text was updated successfully, but these errors were encountered: