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

StreamReader in sendConnect eats half-completed INFO message packets #858

Open
Madgvox opened this issue Jan 31, 2024 · 6 comments
Open
Labels
defect Suspected defect such as a bug or regression

Comments

@Madgvox
Copy link

Madgvox commented Jan 31, 2024

Observed behavior

When I use the NATS client library in a C# console application, it works as expected. When I use it within the Unity game engine, it fails to connect and is stuck in a RECONNECTING state.

This bug requires some context: I am implementing the NATS C# library within the Unity game engine. For some reason, when the connection is upgraded to TLS, the PONG network message and subsequent INFO is truncated to 512 bytes. I have no idea why this happens (and it's not important for this bug report, but it reveals a bug within the library itself).

Here's a mock-up of what I see coming down the wire:

PONG
INFO {<big block of JSON conn

And then after a moment, I get the rest:

ection info>}

I traced the error to the message parser, which was choking on the truncated last portion of the INFO message. I then traced it back to the sendConnect method. Essentially, here's what's happening:

The sendConnect method reads from the stream for the PONG message. However, to accomplish this, it's creating a StreamReader and calling ReadLine() from it. This returns the PONG line, but the StreamReader also consumes the first portion of the INFO message (as seen above). This leaves the rest of the INFO message to be parsed inside the runLoop, which is not in the correct state to parse the message and dies. This repeats ad infinitum as addition PONG/INFO message pairs are sent.

When I replaced these two lines:

sr = new StreamReader(br, Encoding.UTF8, false, MaxControlLineSize, true);
result = sr.ReadLine();

with a hacked together solution that reads the PONG message byte-by-byte:

byte[] buf = new byte[4096];
byte next;
int i = 0;
do {
    next = (byte)br.ReadByte();
    if( next == '\n' || next == 0 ) {
        break;
    }
    buf[i] = next;
    i += 1;
} while( true );
result = Encoding.UTF8.GetString( buf, 0, i );
result = result.Trim();

then the library connected successfully and began operating normally.

In summary, the current logic eats any subsequent messages that follow the PONG message in the same packet. Under normal operating conditions this doesn't appear to have any ill-effects, but under the bizarre 512 limit imposed by the Unity game engine it results in a critical parsing error.

Expected behavior

When I use the NATS client library in the Unity game engine, it works as expected.

Server and client version

N/A to the server/client version
Affects NATS C# 1.1.2

Host environment

This is the C# NATS library being run within the Unity Game Engine environment, on version 2021.3.29.

Steps to reproduce

I haven't determined whether this reliably reproduces the problem because I don't know the RC on Unity's side. But here's the essence of my setup:

  1. Download Unity Game Engine 2021.3.29
  2. Set up a NATS server with TLS required
  3. Import the C# NATS library
  4. Set up a connection to the NATS server. Default options are fine. This was essentially the code I used to test:
using NATS.Client;
using UnityEngine;

public class NetworkTest : MonoBehaviour {
        private IConnection connection;

        public void Start () {
            var opts = ConnectionFactory.GetDefaultOptions();
            opts.Url = "the_url";
            opts.SetUserCredentials( "the_creds_path" );
            connection = new ConnectionFactory().CreateConnection( opts );
       }
}
  1. Run Unity in Play Mode. Assuming this isn't a fluke with my installation and is instead some weird quirk with that Unity version's TLS libraries, the connection should never successfully complete and will be stuck in a RECONNECTING state.
@Madgvox Madgvox added the defect Suspected defect such as a bug or regression label Jan 31, 2024
@Madgvox Madgvox changed the title StreamReader in sendConnect eats half-completed message packets StreamReader in sendConnect eats half-completed INFO message packets Jan 31, 2024
@scottf
Copy link
Collaborator

scottf commented Jan 31, 2024

@Madgvox I just saw this but before I did too deep - is this a pretty new project? If so you might be better off using the .NET V2 client. https://github.com/nats-io/nats.net.v2

Also my first question is can this issue be repeated outside the unity engine? There should be no line break or pause in the INFO json.

@Madgvox
Copy link
Author

Madgvox commented Jan 31, 2024

I just saw this but before I did too deep - is this a pretty new project? If so you might be better off using the .NET V2 client.

I didn't know nats.net.v2 existed, but unfortunately Unity's supported .NET version lags behind the cutting edge and doesn't include some of the language features used by the new library (eg. file-scoped namespaces), so I am stuck using this version.

Also my first question is can this issue be repeated outside the unity engine? There should be no line break or pause in the INFO json.

I repeated a similar test setup in a C# console application, and the issue didn't occur. The INFO json came down in a single packet. To emphasize: I don't believe the INFO json being split between two packets is being caused by the NATS library. It appears to be coming from the TLS layer, the handling of which is being overridden by Unity's own internal stuff, for whatever reason. I have no reason to believe that this bizarre behavior is being caused by either the NATS C# library or the NATS server. I should also mention the detail that the Unity bug only happens with the TLS connection. The first handshake INFO packet comes down in one piece (at ~600 bytes). It's just the TLS PONG and subsequent INFO that gets truncated at 512 bytes.

The bug is that the INFO json is being eaten/ignored by the client in such a way that it breaks if the packet is split between two reads. There's no line break, the whole INFO packet is just not being read all at once. I believe the fix I implemented is the correct way to handle it, as it ensures that the stream is in a valid, parseable state after connect, regardless of the state of the packets. The reason I'm submitting an issue instead of a PR is because my implementation is hacked together garbage.

@scottf
Copy link
Collaborator

scottf commented Jan 31, 2024

There is a recent feature in the 2.10 servers which is supported in the client, called TLS first. You have to configure the server for this, then also tell the connection to do this behavior. Can you see if this helps?

The client:

Options opts = ConnectionFactory.GetDefaultOptions();
...
opts.TlsFirst = true;

The server conf, add to the tls section

tls {
  ...
  handshake_first: 300ms
}

the time being round trip time to do the tls handshake before the server times out the connection try.

@Madgvox
Copy link
Author

Madgvox commented Jan 31, 2024

I'm currently using Synadia as the server backbone. Is there a way to configure tls first with it?

@scottf
Copy link
Collaborator

scottf commented Feb 1, 2024

Can you run a local version to try? Also, can you dm me on slack, might make this conversation easier

@scottf
Copy link
Collaborator

scottf commented Jul 8, 2024

@Madgvox Lost traction on this, but I'm wondering if this PR fixes this problem: https://github.com/nats-io/nats.net/pull/888

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Development

No branches or pull requests

2 participants