WSClient.update should not decode binary data frames

[vereszol]

What happened (please include outputs or screenshots):

https://github.com/kubernetes-client/python/blob/v25.3.0/kubernetes/base/stream/ws_client.py#L198-L203

            elif op_code == ABNF.OPCODE_BINARY or op_code == ABNF.OPCODE_TEXT:
                data = frame.data
                if six.PY3:
                    data = data.decode("utf-8", "replace")
                if len(data) > 1:
                    channel = ord(data[0])

WSClient.update is decoding the received WebSocket data frame even if the opcode indicates a binary payload.
According https://www.rfc-editor.org/rfc/rfc6455#section-5.6 text data frames are encoded as UTF-8, binary is not.

What you expected to happen:

Only text data frames should be decoded. A possible solution would be:

            elif op_code == ABNF.OPCODE_BINARY or op_code == ABNF.OPCODE_TEXT:
                data = frame.data
                if six.PY3 and op_code == ABNF.OPCODE_TEXT:
                    data = data.decode("utf-8", "replace")
                if len(data) > 1:
                    channel = data[0]  # ord() would fail for binary data

How to reproduce it (as minimally and precisely as possible):

Call WSClient.update

Anything else we need to know?:

This change would enable copying of binary files to/from pods, similar to #1471

Environment:

• Kubernetes version (kubectl version):
• OS (e.g., MacOS 10.13.6):
• Python version (python --version): 3.7.3 or newer
• Python client version (pip list | grep kubernetes): 25.3.0

[venukarnati92]

I will take a look at the issue and will make necessary changes.
/assign

[roycaihw]

Thanks for reporting the issue @vereszol. And thanks for contributing @venukarnati92

[venukarnati92]

@vereszol I believe we have to decode the binary data too based on the way it's implemented.

For example an empty string frame will be like this fin=1 opcode=2 data=b'\x01' .So, when I decode the data then only we can find this is an empty string.Then the following code block will be skipped

        if len(data) > 1:
                    channel = ord(data[0])
                    # channel = data[0]
                    data = data[1:]
                    if data:
                        if channel in [STDOUT_CHANNEL, STDERR_CHANNEL]:
                            # keeping all messages in the order they received
                            # for non-blocking call.
                            self._all.write(data)
                        if channel not in self._channels:
                            self._channels[channel] = data
                        else:
                            self._channels[channel] += data

[vereszol]

@venukarnati92 The code block in question would be skipped even without decoding data because len(b'\x01') == 1.

On the other hand, I found that the change I suggested breaks WSClient.readline_channel as it expects decoded data when checking for newline character at if "\n" in data in line #95. That could be fixed by comparing to b'\n' if data is binary.

Unfortunately the end result would anyway be backwards incompatible as it might change the data type of stdout or stderr channel. For this reason I will close the ticket.