Skip to content

BUG: Attach HTTP stream parsing fails if TTY is enabled #97

@ximon18

Description

@ximon18

From the Docker API documentation for attaching to a container:

Stream format
When the TTY setting is disabled in POST /containers/create, the stream over the hijacked connected is multiplexed to separate out stdout and stderr. The stream consists of a series of frames, each containing a header and a payload.

This is supported by dockworker (0.0.18).

And:

Stream format when using a TTY
When the TTY setting is enabled in POST /containers/create, the stream is not multiplexed. The data exchanged over the hijacked connection is simply the raw data from the process PTY and client's stdin.

This second mode is NOT supported by dockworker (0.0.18). What happens is that AttachResponseIter::next() fails because it assumes that the response consists of frames starting with an 8 byte header, but when using ContainerCreateOptions::tty(true) those 8 bytes are actual output from the container, not a header. In my case it interpreted the frame size as a very large number and failed to allocate enough memory for the frame.

I'm not sure how to properly solve this as the stream format doesn't contain a magic value that can be used to detect framed mode, and the knowledge that the container was created with TTY mode is far away from the container attach code which only knows about a container ID. The code leans heavily on implementing standard Rust traits and so the interfaces cannot be changed to include a flag to tell the parsing how to behave. With some refactoring, perhaps this was intended in the improve/attach-api branch?, something can be done I'm sure, but being new to Rust I'm not ready to offer an implementation yet myself.

I have created a proof-of-concept to show tests the first four bytes of the stream header which should always be 0x0000, 0x1000 or 0x2000, and while these are non-printable ASCII codes, I think they represent real UTF-8 code points and so could appear in a multiplexed non-framed UTF-8 stream, i.e. this hack won't work in that case. I didn't want to submit it as a PR, as it doesn't feel like the right solution, but it does show that the problem can be fixed.

See: https://github.com/eldesh/dockworker/compare/master...ximon18:patch-1?expand=1

I tested this patch on a local build of v0.0.18 successfully both with TTY false and TTY true.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions