ssync_sender encoding issue

I'm having trouble duplicating this bug for replicated objects.

In my tests the handling of the object name and metadata in the object server is being treated consistently as utf-8 encoded bytes - not unicode strings.

object-6010: STDOUT: 'Content-Type': 'application/octet-stream'
object-6010: STDOUT: 'ETag': 'd41d8cd98f00b204e9800998ecf8427e'
object-6010: STDOUT: 'X-Object-Meta-Mtime': '1490985870.469097'
object-6010: STDOUT: 'X-Object-Meta-\xe2\x98\x83': '\xe2\x98\x83'
object-6010: STDOUT: 'X-Timestamp': '1490987405.84984'

On master, the EC reconstructor seems to blow up in an entirely different spot:

object-6030: STDERR: Traceback (most recent call last):
object-6030: STDERR: File "/usr/local/lib/python2.7/dist-packages/eventlet/hubs/hub.py", line 457, in fire_timers
object-6030: STDERR: timer()
object-6030: STDERR: File "/usr/local/lib/python2.7/dist-packages/eventlet/hubs/timer.py", line 58, in __call__
object-6030: STDERR: cb(*args, **kw)
object-6030: STDERR: File "/usr/local/lib/python2.7/dist-packages/eventlet/greenthread.py", line 214, in main
object-6030: STDERR: result = function(*args, **kwargs)
object-6030: STDERR: File "/vagrant/swift/swift/common/utils.py", line 2726, in _run_func
object-6030: STDERR: self._responses.put(func(*args, **kwargs))
object-6030: STDERR: File "/vagrant/swift/swift/obj/reconstructor.py", line 231, in _get_response
object-6030: STDERR: full_path = _full_path(node, part, path, policy)
object-6030: STDERR: File "/vagrant/swift/swift/obj/reconstructor.py", line 90, in _full_path
object-6030: STDERR: 'policy': policy,
object-6030: STDERR: UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 20: ordinal not in range(128)

I'm creating objects from the command line using u'\N{SNOWMAN}' in the name & metadata

swift upload test ☃ -H 'x-object-meta-☃: ☃'

There's definitely something going on with ec sync and non-ascii names, but the associated change doesn't 100% square it for me yet:

https://review.openstack.org/#/c/452112/

The EC sysmeta metadata items, whose keys are unicode, are those derived from footers sent at end of EC frags[1]. The footers are serialized JSON and when the footer string is loaded in object server PUT path with json.loads() all key/value pairs are returned unicode (thats what JSON generates when parsing). The sysmeta from footers is then added to the object metadata[2], hence some unicode keys in metadata

Presumably, when serializing the metadata to send via ssync, once unicode is encountered by a join() then any ascii will be decoded to unicode assuming default ascii encoding and any non-ascii characters will then trigger an exception.

[1] https://github.com/openstack/swift/blob/11678e2f751fb77f10db2357ed886c846e3f87b3/swift/proxy/controllers/obj.py#L1842-L1868
[2] https://github.com/openstack/swift/blob/11678e2f751fb77f10db2357ed886c846e3f87b3/swift/obj/server.py#L796-L808

WRT Clay's comment #1, the _full_path method will blow up when path arg has non-ascii chars because the node dict items are unicode, so the string format will try to coerce the path to unicode.

The node dict is all unicode because it is loaded using json.loads() in RingData.load()

The call to _full_path in get_response was until recently only made when there was a warning to log, but now _full_path is called unconditionally at start of get_response [1].

[1] https://review.openstack.org/#/c/219165/60/swift/obj/reconstructor.py@222

I think that what Clay reported in #1 is actually a different bug that was made more visible since Ocata release. I raised a separate bug report for it:

https://bugs.launchpad.net/swift/+bug/1679175

I propose making diskfile persist all metadata keys and values as either bytes or unicode. All items apart from the footer metadata are already persisted as bytes. The footer metadata values are encoded to bytes by HeaderKeyDict that they are stored in by the object server.

So that leaves just the footer keys that by virtue of json parsing are unicode. We could have the object server encode those keys. That would fix things going forwards but still leave a problem with existig diskfiles with unicode keys in metadata. So I propose that diskfile should encode in both the write_metadata() (going forwards) and read_metadata() (for legacy metadata). We could of course just encode in read_metadata() but it seems less confusing going forwards to have a uniform type persisted in diskfile.

I cannot reproduce this bug as reported i.e. with non-ascii characters in an object name, but I can get the symptoms by putting non-ascii chars in an x-object-meta metadata value [1]

The object path is quoted here [2] before being passed to send_put method so it is fine to decode as utf8.

But if there is non-ascii in a metadata value AND a metadata key that is unicode (as we have seen EC writes into diskfle) then the send_put will blow up when it attempts to join those into a string, at the same line as reported, but the "culprit" is not the object name.

[1]
Apr 7 18:32:12 localhost object-reconstructor: 127.0.0.1:6020/sdb6/390 EXCEPTION in ssync.Sender: #012Traceback (most recent call last):#012 File "/home/swift/swift/swift/obj/ssync_sender.py", line 126, in __call__#012 self.updates()#012 File "/home/swift/swift/swift/obj/ssync_sender.py", line 346, in updates#012 self.send_put(url_path, df_alt)#012 File "/home/swift/swift/swift/obj/ssync_sender.py", line 410, in send_put#012 msg = '\r\n'.join(msg) + '\r\n\r\n'#012UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 28: ordinal not in range(128)

[2] https://github.com/openstack/swift/blob/cff7455a689e452d12e85eec69137b3a3e0ec803/swift/obj/ssync_sender.py#L335-L336

Like clay, I put snowmen everywhere: http://paste.openstack.org/show/606008/

And was also getting _full_path issues, and ssync sender stuff.

This patch seems to fix it for me. It makes sure the metadata from the diskfile is encoded to utf8 so it is handled the same in repl and EC (thanks to Alistair's notes).

And then decoded to unicode in _full_path so we don't get errors there.

I'm sure there are better ways, but this seems to work and it hasn't broken any unit tests.. although I haven't yet created a unit test that fails. This also gets around not needing to have the key utf8 string-ified (current gerrit patch), because the metadata has already been encoded.

It may be overkill, but this is where I got to this arvo, and I need to finish up now (dinner). So haven't simplified further.

Change abandoned by Alistair Coles (<email address hidden>) on branch: master
Review: https://review.openstack.org/454860
Reason: squashed into https://review.openstack.org/#/c/452112/2

Reviewed: https://review.openstack.org/452112
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=091157fc7f52acfeb24e87e26b7f142da9e50d6b
Submitter: Jenkins
Branch: master

commit 091157fc7f52acfeb24e87e26b7f142da9e50d6b
Author: Romain LE DISEZ <email address hidden>
Date: Fri Mar 31 11:21:13 2017 +0200

    Fix encoding issue in ssync_sender.send_put()

    EC object metadata can currently have a mixture of bytestrings and
    unicode. The ssync_sender.send_put() method raises an
    UnicodeDecodeError when it attempts to concatenate the metadata
    values, if any bytestring has non-ascii characters.

    The root cause of this issue is that the object server uses unicode
    for the keys of some object metadata items that are received in the
    footer of an EC PUT request, whereas all other object metadata keys
    and values are persisted as bytestrings.

    This patch fixes the bug by changing diskfile write_metadata()
    function to encode all unicode metadata keys and values as utf8
    encoded bytes before writing to disk. To cope with existing objects
    that have a mixture of unicode and bytestring metadata, the diskfile
    read_metadata() function is also changed so that all returned unicode
    metadata keys and values are utf8 encoded. This ensures that
    ssync_sender.send_put() (and any other caller of diskfile
    read_metadata) only reads bytestrings from object metadata.

    Closes-Bug: #1678018
    Co-Authored-By: Alistair Coles <email address hidden>
    Change-Id: Ic23c55754ee142f6f5388dcda592a3afc9845c39

This issue was fixed in the openstack/swift 2.14.0 release.