Skip to content

Improve ShellStream Expect #1315

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 11, 2024
Merged

Improve ShellStream Expect #1315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions src/Renci.SshNet/ShellStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,9 @@ public void Expect(TimeSpan timeout, params ExpectAction[] expectActions)
if (match.Success)
{
var result = text.Substring(0, match.Index + match.Length);
var charCount = _encoding.GetByteCount(result);

for (var i = 0; i < match.Index + match.Length && _incoming.Count > 0; i++)
for (var i = 0; i < charCount && _incoming.Count > 0; i++)
{
// Remove processed items from the queue
_ = _incoming.Dequeue();
Expand Down Expand Up @@ -348,23 +349,26 @@ public string Expect(Regex regex)
/// </returns>
public string Expect(Regex regex, TimeSpan timeout)
{
var text = string.Empty;
var result = string.Empty;

while (true)
{
lock (_incoming)
{
if (_incoming.Count > 0)
{
text = _encoding.GetString(_incoming.ToArray(), 0, _incoming.Count);
result = _encoding.GetString(_incoming.ToArray(), 0, _incoming.Count);
}

var match = regex.Match(text);
var match = regex.Match(result);

if (match.Success)
{
result = result.Substring(0, match.Index + match.Length);
var charCount = _encoding.GetByteCount(result);

// Remove processed items from the queue
for (var i = 0; i < match.Index + match.Length && _incoming.Count > 0; i++)
for (var i = 0; i < charCount && _incoming.Count > 0; i++)
{
_ = _incoming.Dequeue();
}
Expand All @@ -386,7 +390,7 @@ public string Expect(Regex regex, TimeSpan timeout)
}
}

return text;
return result;
}

/// <summary>
Expand Down Expand Up @@ -471,6 +475,7 @@ public IAsyncResult BeginExpect(TimeSpan timeout, AsyncCallback callback, object
if (match.Success)
{
var result = text.Substring(0, match.Index + match.Length);
var charCount = _encoding.GetByteCount(result);

for (var i = 0; i < match.Index + match.Length && _incoming.Count > 0; i++)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this line use charCount?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor

@jscarle jscarle Feb 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I fixed it in my PR.

{
Expand Down
25 changes: 10 additions & 15 deletions test/Renci.SshNet.Tests/Classes/ShellStreamTest_ReadExpect.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,6 @@ public void ReadLine_NoData_ReturnsNullAfterClose()
}

[TestMethod]
[Ignore] // Fails because it returns the whole buffer i.e. "Hello World!\r\n12345"
// We might actually want to keep that behaviour, but just make the documentation clearer.
// The Expect documentation says:
// "The text available in the shell that contains all the text that ends with expected expression."
// Does that mean
// 1. the returned string ends with the expected expression; or
// 2. the returned string is all the text in the buffer, which is guaranteed to contain the expected expression?
// The current behaviour is closer to 2. I think the documentation implies 1.
// Either way, there are bugs.
public void Expect()
{
_channelSessionStub.Receive(Encoding.UTF8.GetBytes("Hello "));
Expand All @@ -181,10 +172,6 @@ public void Expect()
// Case 1 above.
Assert.AreEqual("Hello World!\r\n123", _shellStream.Expect("123")); // Fails, returns "Hello World!\r\n12345"
Assert.AreEqual("45", _shellStream.Read()); // Passes, but should probably fail and return ""

// Case 2 above.
Assert.AreEqual("Hello World!\r\n12345", _shellStream.Expect("123")); // Passes
Assert.AreEqual("", _shellStream.Read()); // Fails, returns "45"
}

[TestMethod]
Expand Down Expand Up @@ -213,7 +200,6 @@ public void ReadLine_MultiByte()
}

[TestMethod]
[Ignore]
public void Expect_Regex_MultiByte()
{
_channelSessionStub.Receive(Encoding.UTF8.GetBytes("𐓏𐓘𐓻𐓘𐓻𐓟 𐒻𐓟"));
Expand All @@ -223,7 +209,6 @@ public void Expect_Regex_MultiByte()
}

[TestMethod]
[Ignore]
public void Expect_String_MultiByte()
{
_channelSessionStub.Receive(Encoding.UTF8.GetBytes("hello 你好"));
Expand All @@ -232,6 +217,16 @@ public void Expect_String_MultiByte()
Assert.AreEqual("", _shellStream.Read());
}

[TestMethod]
public void Expect_String_non_ASCII_characters()
{
_channelSessionStub.Receive(Encoding.UTF8.GetBytes("Hello, こんにちは, Bonjour"));

Assert.AreEqual("Hello, こ", _shellStream.Expect(new Regex(@"[^\u0000-\u007F]")));

Assert.AreEqual("んにちは, Bonjour", _shellStream.Read());
}

[TestMethod]
public void Expect_Timeout()
{
Expand Down