Skip to content

Commit

Permalink
Fix handling of non-multiplexed (TTY) streams over upgraded sockets
Browse files Browse the repository at this point in the history
Signed-off-by: Joffrey F <[email protected]>
  • Loading branch information
shin- committed Aug 17, 2017
1 parent a6065df commit d9df2a8
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 11 deletions.
22 changes: 16 additions & 6 deletions docker/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
from ..tls import TLSConfig
from ..transport import SSLAdapter, UnixAdapter
from ..utils import utils, check_resource, update_headers
from ..utils.socket import frames_iter
from ..utils.socket import frames_iter, socket_raw_iter
from ..utils.json_stream import json_stream
try:
from ..transport import NpipeAdapter
Expand Down Expand Up @@ -362,13 +362,19 @@ def _stream_raw_result(self, response):
for out in response.iter_content(chunk_size=1, decode_unicode=True):
yield out

def _read_from_socket(self, response, stream):
def _read_from_socket(self, response, stream, tty=False):
socket = self._get_raw_response_socket(response)

gen = None
if tty is False:
gen = frames_iter(socket)
else:
gen = socket_raw_iter(socket)

if stream:
return frames_iter(socket)
return gen
else:
return six.binary_type().join(frames_iter(socket))
return six.binary_type().join(gen)

def _disable_socket_timeout(self, socket):
""" Depending on the combination of python version and whether we're
Expand Down Expand Up @@ -398,9 +404,13 @@ def _disable_socket_timeout(self, socket):

s.settimeout(None)

def _get_result(self, container, stream, res):
@check_resource('container')
def _check_is_tty(self, container):
cont = self.inspect_container(container)
return self._get_result_tty(stream, res, cont['Config']['Tty'])
return cont['Config']['Tty']

def _get_result(self, container, stream, res):
return self._get_result_tty(stream, res, self._check_is_tty(container))

def _get_result_tty(self, stream, res, is_tty):
# Stream multi-plexing was only introduced in API v1.6. Anything
Expand Down
4 changes: 3 additions & 1 deletion docker/api/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ def attach(self, container, stdout=True, stderr=True,
u = self._url("/containers/{0}/attach", container)
response = self._post(u, headers=headers, params=params, stream=stream)

return self._read_from_socket(response, stream)
return self._read_from_socket(
response, stream, self._check_is_tty(container)
)

@utils.check_resource('container')
def attach_socket(self, container, params=None, ws=False):
Expand Down
2 changes: 1 addition & 1 deletion docker/api/exec_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,4 @@ def exec_start(self, exec_id, detach=False, tty=False, stream=False,
return self._result(res)
if socket:
return self._get_raw_response_socket(res)
return self._read_from_socket(res, stream)
return self._read_from_socket(res, stream, tty)
21 changes: 20 additions & 1 deletion docker/utils/socket.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,24 @@ def frames_iter(socket):
break
while n > 0:
result = read(socket, n)
n -= len(result)
if result is None:
continue
data_length = len(result)
if data_length == 0:
# We have reached EOF
return
n -= data_length
yield result


def socket_raw_iter(socket):
"""
Returns a generator of data read from the socket.
This is used for non-multiplexed streams.
"""
while True:
result = read(socket)
if len(result) == 0:
# We have reached EOF
return
yield result
2 changes: 1 addition & 1 deletion tests/integration/api_build_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ def test_build_with_network_mode(self):
with pytest.raises(errors.NotFound):
self.client.inspect_image('dockerpytest_nonebuild')

@requires_experimental(until=None)
@requires_api_version('1.25')
@requires_experimental
def test_build_squash(self):
script = io.BytesIO('\n'.join([
'FROM busybox',
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def fake_delete(self, url, *args, **kwargs):
return fake_request('DELETE', url, *args, **kwargs)


def fake_read_from_socket(self, response, stream):
def fake_read_from_socket(self, response, stream, tty=False):
return six.binary_type()


Expand Down

0 comments on commit d9df2a8

Please sign in to comment.