Skip to content
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

Update dependencies and create new session on reconnect rather than trying to recreate session #2375

Merged
merged 8 commits into from
Feb 25, 2025

Conversation

marcschier
Copy link
Collaborator

No description provided.

@marcschier marcschier requested a review from a team as a code owner February 24, 2025 14:06
@marcschier
Copy link
Collaborator Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -921,22 +969,25 @@
{
case SessionState.Connected: // only valid when connected.
Debug.Assert(_reconnectHandler.State == SessionReconnectHandler.ReconnectState.Ready);
_logger.LogInformation("{Client}: Reconnecting session {Session} due to {Reason}...",
this, _sessionName, (context is ServiceResult sr) ? "error " + sr : "RESET");

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.

Copilot Autofix AI about 4 hours ago

To fix the problem, we need to sanitize the user input before logging it. Specifically, we should remove any newline characters from the _sessionName variable to prevent log forging. This can be done using the String.Replace method to replace newline characters with an empty string.

The best way to fix the problem without changing existing functionality is to modify the logging statements to sanitize the _sessionName variable. This involves updating the logging statements on lines 973 and 979 to use a sanitized version of _sessionName.

Suggested changeset 1
src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs b/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs
--- a/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs
+++ b/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs
@@ -971,4 +971,5 @@
                                             Debug.Assert(_reconnectHandler.State == SessionReconnectHandler.ReconnectState.Ready);
+                                            var sanitizedSessionName = _sessionName?.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
                                             _logger.LogInformation("{Client}: Reconnecting session {Session} due to {Reason}...",
-                                                this, _sessionName, (context is ServiceResult sr) ? "error " + sr : "RESET");
+                                                this, sanitizedSessionName, (context is ServiceResult sr) ? "error " + sr : "RESET");
 
@@ -978,3 +979,3 @@
                                             _logger.LogInformation("{Client}: Begin reconnecting session {Session}...",
-                                                this, _sessionName);
+                                                this, sanitizedSessionName);
                                             Debug.Assert(_session != null);
EOF
@@ -971,4 +971,5 @@
Debug.Assert(_reconnectHandler.State == SessionReconnectHandler.ReconnectState.Ready);
var sanitizedSessionName = _sessionName?.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
_logger.LogInformation("{Client}: Reconnecting session {Session} due to {Reason}...",
this, _sessionName, (context is ServiceResult sr) ? "error " + sr : "RESET");
this, sanitizedSessionName, (context is ServiceResult sr) ? "error " + sr : "RESET");

@@ -978,3 +979,3 @@
_logger.LogInformation("{Client}: Begin reconnecting session {Session}...",
this, _sessionName);
this, sanitizedSessionName);
Debug.Assert(_session != null);
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
_logger.LogInformation("{Client}: Reconnecting session {Session} due to {Reason}...",
this, _sessionName, (context is ServiceResult sr) ? "error " + sr : "RESET");
_logger.LogInformation("{Client}: Begin reconnecting session {Session}...",
this, _sessionName);

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.

Copilot Autofix AI about 4 hours ago

To fix the problem, we need to sanitize the user input before logging it. Specifically, we should remove any newline characters from the _sessionName to prevent log forging. This can be done using the Replace method to ensure that no new lines are present in the user input.

  1. Identify the lines where _sessionName is logged.
  2. Sanitize _sessionName by replacing newline characters with an empty string before logging it.
  3. Ensure that the sanitization does not alter the existing functionality of the application.
Suggested changeset 1
src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs b/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs
--- a/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs
+++ b/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs
@@ -972,3 +972,3 @@
                                             _logger.LogInformation("{Client}: Reconnecting session {Session} due to {Reason}...",
-                                                this, _sessionName, (context is ServiceResult sr) ? "error " + sr : "RESET");
+                                                this, _sessionName?.Replace(Environment.NewLine, ""), (context is ServiceResult sr) ? "error " + sr : "RESET");
 
@@ -978,3 +978,3 @@
                                             _logger.LogInformation("{Client}: Begin reconnecting session {Session}...",
-                                                this, _sessionName);
+                                                this, _sessionName?.Replace(Environment.NewLine, ""));
                                             Debug.Assert(_session != null);
EOF
@@ -972,3 +972,3 @@
_logger.LogInformation("{Client}: Reconnecting session {Session} due to {Reason}...",
this, _sessionName, (context is ServiceResult sr) ? "error " + sr : "RESET");
this, _sessionName?.Replace(Environment.NewLine, ""), (context is ServiceResult sr) ? "error " + sr : "RESET");

@@ -978,3 +978,3 @@
_logger.LogInformation("{Client}: Begin reconnecting session {Session}...",
this, _sessionName);
this, _sessionName?.Replace(Environment.NewLine, ""));
Debug.Assert(_session != null);
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@@ -962,6 +1013,8 @@
switch (currentSessionState)
{
case SessionState.Reconnecting:
_logger.LogInformation("{Client}: Completed reconnecting session {Session}...",
this, _sessionName);

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.

Copilot Autofix AI about 4 hours ago

To fix the problem, we need to sanitize the _sessionName variable before logging it. This can be done by removing any newline characters from the user input to prevent log forging. We will use the Replace method to remove newline characters from _sessionName before it is logged.

  • Identify the line where _sessionName is logged.
  • Sanitize _sessionName by replacing newline characters with an empty string.
  • Ensure that the sanitization does not alter the existing functionality of the code.
Suggested changeset 1
src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs b/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs
--- a/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs
+++ b/src/Azure.IIoT.OpcUa.Publisher/src/Stack/Services/OpcUaClient.cs
@@ -1015,4 +1015,5 @@
                                         case SessionState.Reconnecting:
+                                            var sanitizedSessionName = _sessionName?.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
                                             _logger.LogInformation("{Client}: Completed reconnecting session {Session}...",
-                                                this, _sessionName);
+                                                this, sanitizedSessionName);
                                             //
EOF
@@ -1015,4 +1015,5 @@
case SessionState.Reconnecting:
var sanitizedSessionName = _sessionName?.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
_logger.LogInformation("{Client}: Completed reconnecting session {Session}...",
this, _sessionName);
this, sanitizedSessionName);
//
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@marcschier marcschier merged commit eee22fb into main Feb 25, 2025
4 of 7 checks passed
@marcschier marcschier deleted the updates branch February 25, 2025 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant