Ensure to write str(not bin) objects, for compatibility reason #1413
Conversation
|
@repeatedly could you check this fix? |
| sock.write [0xdb, chunk_io.size].pack('CN') # 2. beginRaw(size) raw32 | ||
| IO.copy_stream(chunk_io, sock) # writeRawBody(packed_es) | ||
| entries = [0xdb, chunk_io.size].pack('CN') | ||
| sock.write entries.force_encoding(Encoding::UTF_8) # 2. entries: String (str32) |
There was a problem hiding this comment.
If the entries are compressed, we should not use UTF-8 for such content.
There was a problem hiding this comment.
It's true for any entries objects, but it's for compatibility.
If we pack it as ASCII-8BIT, Fluentd v0.14 will pack it as bin data, and will break v0.12 Fluentd.
There was a problem hiding this comment.
We can't use compressed mode with fluentd v0.12 because v0.12's in_forward doesn't support compressed mode.
So it seems no problem for me.
There was a problem hiding this comment.
No, this code is for both of compressed and uncompressed transfer mode (by passing compressed: false above).
There was a problem hiding this comment.
Yes. We should change encoding by compressed value.
There was a problem hiding this comment.
This code (packing header, not entry itself) doesn't contain compressed/uncompressed content body. It depends on chunk_io below.
Do you mean that we should take care about the class of chunk and switch 0xdb to 0xc6? I think it's nonsense because It increases code complexity and doesn't compatibility problem because we can't throw away str types for uncompressed data.
There was a problem hiding this comment.
Ah, I missed the code. I thought entries means content body.
And I noticed the commde. After drop older msgpack, we will change it to bin for binary data?
There was a problem hiding this comment.
Yes. I added a comment for it.
https://github.com/fluent/fluentd/pull/1413/files#diff-40c0e8c88efdc9a297a9fc63bc2bc69fR550
fixes #1406.