Skip to content

Skip null data (fix NRE) in output data received handler #11448

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
daxian-dbw merged 9 commits into from
Jan 10, 2020
Merged

Skip null data (fix NRE) in output data received handler #11448

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5335323
Skip null data in output data received handler
iSazonov Dec 28, 2019
a087950
Move fix to GetMessageGuid()
iSazonov Jan 6, 2020
c893e62
Refactor GetMessageGuid() to avoid extra allocations
iSazonov Jan 6, 2020
69522d5
Address feedback
iSazonov Jan 7, 2020
bedfef8
Implement IsCommandMessage()
iSazonov Jan 7, 2020
0cc9c70
Remove comment
iSazonov Jan 7, 2020
7e2a097
Move if block out of try-catch
iSazonov Jan 7, 2020
20edc1e
Implement IsSessionMessage()
iSazonov Jan 7, 2020
6f49088
Inline IsSessionMessage()
iSazonov Jan 7, 2020
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
39 changes: 12 additions & 27 deletions src/System.Management.Automation/engine/remoting/fanin/OutOfProcTransportManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ internal override void Dispose(bool isDisposing)
{
// Object already disposed.
}

_commandMessageQueue.Dispose();
}
}
Expand Down Expand Up @@ -741,41 +741,26 @@ private void ProcessMessageProc(object state)
}
}

private const string GUIDTAG = "PSGuid='";
private const int GUID_STR_LEN = 36; // GUID string: 32 digits plus 4 dashes

private Guid GetMessageGuid(string data)
{
// Perform quick scan for data packet for a GUID, ignoring any errors.
var iTag = data.IndexOf(GUIDTAG, StringComparison.OrdinalIgnoreCase);
if (iTag > -1)
{
try
{
var psGuidString = data.Substring(iTag + GUIDTAG.Length, GUID_STR_LEN);
return new Guid(psGuidString);
}
catch
{
// Ignore any malformed packet errors here and return an empty Guid.
// Packet errors will be reported later during message processing.
}
}

return Guid.Empty;
}

#endregion

#region Event Handlers

private const string SESSIONDMESSAGETAG = "PSGuid='00000000-0000-0000-0000-000000000000'";

protected void HandleOutputDataReceived(string data)
{
if (string.IsNullOrEmpty(data))
{
// A null/empty data string indicates a problem in the transport,
// e.g., named pipe emitting a null packet because it closed or some reason.
// In this case we simply ignore the packet.
return;
}

try
{
// Route protocol message based on whether it is a session or command message.
Copy link
Contributor

@PaulHigin PaulHigin Jan 6, 2020

Choose a reason for hiding this comment

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

Please move string null/empty check to here.

Copy link
Collaborator Author

@iSazonov iSazonov Jan 7, 2020

Choose a reason for hiding this comment

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

I added the explicit check.

// Session messages have empty Guid values.
if (Guid.Equals(GetMessageGuid(data), Guid.Empty))
if (data.IndexOf(SESSIONDMESSAGETAG, StringComparison.OrdinalIgnoreCase) > -1)
{
// Session message
_sessionMessageQueue.Add(data);
Expand Down