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

SQL Server has closed connection unexpectedly if sql request size more then package size and it is chunked (probably only together with ssl) #1433

Closed
temaleva opened this issue Apr 15, 2024 · 4 comments
Assignees
Labels
Milestone

Comments

@temaleva
Copy link

temaleva commented Apr 15, 2024

Version

At least version 4.x and actual snapshot 5.0.0-SNAPSHOT

Context

By trying to send sql request (especially insert) which exceeds in size current package size

Reproducer

Reproducer quarkus project could be found here https://github.com/eagely/vertx-closed-connection/tree/master

Steps to reproduce

  1. set tds driver package size small enough to be as small as you request size ( for example using quarkus.datasource.reactive.mssql.packet-size property in quarkus based project).
    The minimal package size is 512 , so you request size must be at least 512 bytes.
  2. Try to execute insert statement
  3. Receive ClosedConnectionException instead of answer from sql server.

Extra

It looks like the problem is connected with tls package size, which must be synchronized with request package size.
The same error was detected (and fixed) in several other mssql clients/libraries, such as .NET and Node.js
At least this one looks very similar tediousjs/tedious#923

It could be, that the problem only appeares on linux, but this is also mean in docker env reproducible as well..
Reproducer use sql docker and was started on linux only. (Not know if on windows host the same exception appears).

@temaleva temaleva added the bug label Apr 15, 2024
@tsegismont tsegismont self-assigned this Apr 15, 2024
@tsegismont tsegismont added this to the 4.5.8 milestone Apr 15, 2024
@tsegismont
Copy link
Contributor

Thanks for the report. I will look into it

@temaleva
Copy link
Author

@tsegismont Could I help somehow? I was trying to understand how one can synchronize ssl package size with tds package size, but was no able to cope it. I believe, based on the fixes in another projects, this should be the solution. Maybe if you can give me some insides on this, so I can try it once again?

@tsegismont
Copy link
Contributor

I was able to reproduce with this test in MSSQLEncryptionTest:

  @Test
  public void testSmallerPacketSize(TestContext ctx) {
    setOptions(rule.options()
      .setSsl(true)
      .setSslOptions(new ClientSSLOptions().setTrustAll(true))
      .setPacketSize(512));

    char[] chars = new char[200];
    Arrays.fill(chars, 'a');
    String str = new String(chars);

    connect(ctx.asyncAssertSuccess(conn -> {
      conn.query("CREATE TABLE #TestX (text NVARCHAR(MAX))").execute().onComplete(ctx.asyncAssertSuccess(v -> {
        conn.preparedQuery("INSERT INTO Test2 (text) VALUES (@p1)").execute(Tuple.of(str)).onComplete(ctx.asyncAssertSuccess());
      }));
    }));
  }

Working on the fix

tsegismont added a commit to tsegismont/vertx-sql-client that referenced this issue May 23, 2024
See eclipse-vertx#1433

When TLS fragments are bigger than the negotiated TDS packet size, the server may close the connection abruptly.

On TLS fragments see https://datatracker.ietf.org/doc/html/rfc5246#section-6.2.1

Signed-off-by: Thomas Segismont <[email protected]>
tsegismont added a commit that referenced this issue May 23, 2024
* Align TLS fragments size with negotiated TDS packet size

See #1433

When TLS fragments are bigger than the negotiated TDS packet size, the server may close the connection abruptly.

On TLS fragments see https://datatracker.ietf.org/doc/html/rfc5246#section-6.2.1

Signed-off-by: Thomas Segismont <[email protected]>

* Remove redundant packetSize field from MSSQLSocketConnection

The desired packet size is already stored in connect options.

Signed-off-by: Thomas Segismont <[email protected]>

---------

Signed-off-by: Thomas Segismont <[email protected]>
tsegismont added a commit that referenced this issue May 23, 2024
* Align TLS fragments size with negotiated TDS packet size

See #1433

When TLS fragments are bigger than the negotiated TDS packet size, the server may close the connection abruptly.

On TLS fragments see https://datatracker.ietf.org/doc/html/rfc5246#section-6.2.1

Signed-off-by: Thomas Segismont <[email protected]>

* Remove redundant packetSize field from MSSQLSocketConnection

The desired packet size is already stored in connect options.

Signed-off-by: Thomas Segismont <[email protected]>

---------

Signed-off-by: Thomas Segismont <[email protected]>
@tsegismont
Copy link
Contributor

Fixed by df39408

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants