Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Adds Support for TCP Hijack in execStartStream #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bosunski wants to merge 7 commits into clue:1.x
base: 1.x
Choose a base branch
Loading
from phpsandbox:writable

Conversation

@bosunski
Copy link

@bosunski bosunski commented Jul 27, 2020

Summary of changes

  • Adds $hijak parameter to exectStartsStream
  • Configures the browser based on $hijak

Example Usage

$client->execCreate('bash', 'bash', true, true, true, true)
 ->then(function($info) use ($client) {
 $client->execStartStream($info['Id'], true,'stderr', true)->then(function (UpgradedResponse $e) use ($deferred) {
 $stream = $e->getConnection();
 $stream->on('data', function ($chunk) {
 echo $chunk;
 });
 $stream->write("ls\r"); // list directories
 });
 });

Copy link
Owner

@clue clue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bosunski Thanks for looking into this, I think this is an excellent first step to discuss this feature in mroe detail!

I love the feature, but I'm not sure about the API in its current form.

The execStartStream() currently returns a ReadableStreamInterface (read-only).

It's my understanding the new API that supports writable streams should return a DuplexStreamInterface instead.

The DuplexStreamInterface extends the ReadableStreamInterface, so we could make this the return value of the same function or it might be better to come up with a dedicated API endpoint, I'm currently undecided.

What do you think about this? 👍

Copy link
Author

bosunski commented Aug 20, 2020 •
edited
Loading

@clue I'm thinking 🤔 since the Hijak feature is still part of the /exec/{exec}/start API and only surfaces when the Hijak is configured, we can just have the execStartStream return a DuplexStreamInterface which, as you've said, will cover for the ReadableStreamInterface also.

I initially added execStartStreamUpgrade as a public interface, but I noticed the same parameters are being passed for it as well as execStartStream which is more like some repetition there. Which lead me to merge the 2 methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clue clue clue left review comments

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support writable streams (process STDIN)

2 participants

AltStyle によって変換されたページ (->オリジナル) /