SQLConnectionStringBuilder 5.0 is not Backwards Compatible

[PeteSL]

Pull request 1608 states that the Encrypt property is backwards compatible with boolean interfaces. It is not. Microsoft.SqlServer.Management.Smo.Server("servername") constructor is failing with "Method not found: 'Void Microsoft.Data.SqlClient.SqlConnectionStringBuilder.set_Encrypt(Boolean)'." during the construction. This is a bug as 1608 states it is backwards compatible. Rolling back to 4.1.0 fixes the issue, as expected.

[lcheunglci]

Thanks @PeteSL for bringing that up. If you pass the entire connection string into the SqlConnectionStringBuilder constructor, it will do the conversion from Encrypt=false to the enum SqlConnectionEncryptOption.Optional and from Encrypt=true to SqlConnectionEncryptOption.Manual to keep it backwards compatible in that perspective. It seems that you are explicitly setting it as a boolean to SqlConnectionStringBuilder.Encrypt property which was replaced from a boolean to the enum SqlConnectionEncryptOption, so in that regards, it a breaking change from 4.1.0 to 5.0.0. In order to add that type of backward compatibility, we'll have to confirm with @David-Engel and @JRahnama if that is also a type of backward compatibility that we would like to support moving forward. We get back to you soon.

[ErikEJ]

SMO needs to be updated to support 5.0

[PeteSL]

@lcheunglci I understand but I have no control over what methods the Microsoft.SqlServer.Management.Smo.Server class calls (Microsoft owns this). Repurposing a property without supporting prior methods is a bad idea, especially when the methods are used by other Microsoft classes. Until SQLClient or Smo.Server are brought into compliance, I will have to leave SQLClient at 4.1.0.

[David-Engel]

Thanks @PeteSL for bringing that up. If you pass the entire connection string into the SqlConnectionStringBuilder constructor, it will do the conversion from Encrypt=false to the enum SqlConnectionEncryptOption.Optional and from Encrypt=true to SqlConnectionEncryptOption.Manual to keep it backwards compatible in that perspective. It seems that you are explicitly setting it as a boolean to SqlConnectionStringBuilder.Encrypt property which was replaced from a boolean to the enum SqlConnectionEncryptOption, so in that regards, it a breaking change from 4.1.0 to 5.0.0. In order to add that type of backward compatibility, we'll have to confirm with @David-Engel David Engel (Simba Technologies Inc) Vendor and @JRahnama Javad Rahnama (Simba Technologies Inc) Vendor if that is also a type of backward compatibility that we would like to support moving forward. We get back to you soon.

The intention was to keep the connection string backwards compatible. Meaning if Encrypt is set to either Optional or Mandatory, or False or True, SqlConnectionStringBuilder would emit Encrypt=False or Encrypt=True. I thought I saw functional tests covering that scenario, but obviously it's not working right. We should address it in a servicing release.

[ErikEJ]

This test passes on latest main:

        [Fact]
        public void ConnectionBuilderEncryptBackwardsCompatibility()
        {
            SqlConnectionStringBuilder builder = new();
            builder.Encrypt = false;
            Assert.Equal("Encrypt=False", builder.ConnectionString);
            Assert.False(builder.Encrypt);
            
            builder.Encrypt = true;
            Assert.Equal("Encrypt=True", builder.ConnectionString);
            Assert.True(builder.Encrypt);

            builder.Encrypt = SqlConnectionEncryptOption.Optional;
            Assert.Equal("Encrypt=False", builder.ConnectionString);
            Assert.False(builder.Encrypt);

            builder.Encrypt = SqlConnectionEncryptOption.Mandatory;
            Assert.Equal("Encrypt=True", builder.ConnectionString);
            Assert.True(builder.Encrypt);
        }

[David-Engel]

That makes me wonder how SMO is calling our methods. There is an implicit conversion defined for SqlConnectionEncryptOption to/from a bool that was meant to cover backwards compatibility.

[ajcvickers]

It's a binary break, but not a source break. Old code recompiled against the new method will work. Old binaries are bound against the old method signature, and hence will break.

[ErikEJ]

SMO should update to 5.0 !?

[David-Engel]

It's a binary break, but not a source break. Old code recompiled against the new method will work. Old binaries are bound against the old method signature, and hence will break.

Ah, right. I'll make a change to the release notes to that effect. Thanks!

[shueybubbles]

When we bump SMO's major version (post SQL 2022 GA) we'll also:
microsoft/sqlmanagementobjects#85
microsoft/sqlmanagementobjects#102
microsoft/sqlmanagementobjects#95

[David-Engel]

Closing this issue. The backwards compatibility issue is not with MDS connection strings, but with the fact that MDS 5.0 is not binary compatible with existing libraries, like SMO. Their code should be backwards compatible, but they need to be recompiled against MDS 5.0.

[crokusek]

I have new code compiled against these as configured by NuGet:
Microsoft.Data.SqlClient 5.1.1
Microsoft.Data.SqlClient.SNI 5.1.0

But was getting the Method Not Found for encrypt at runtime. Had to change source like so:

#if false
_server = new Server(serverName);
#else
// After upgrade the extemely simple "new Server(serverName)" was failing due to an encrypt flag issue not being backward compatible
// #1702
//
// As a workaround, use this explicit construction.
//
SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder();
builder.Encrypt = false;
builder.IntegratedSecurity = true;
builder.DataSource = serverName;

        string conString = builder.ConnectionString;

        _server = new Server(new ServerConnection(new SqlConnection(conString)));

#endif