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.

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

Draft | Addressing the issue with wrong exception when column decryption fails. #1897

Closed
wants to merge 3 commits into from
Closed

Draft | Addressing the issue with wrong exception when column decryption fails. #1897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4eccae
first try
Jan 23, 2023
6f54656
reruning the pipelines
Jan 24, 2023
180d081
rerun
Jan 24, 2023
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
9 changes: 6 additions & 3 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1962,7 +1962,8 @@ internal static bool IsValidTdsToken(byte token)
token == TdsEnums.SQLSSPI ||
token == TdsEnums.SQLFEATUREEXTACK ||
token == TdsEnums.SQLSESSIONSTATE ||
token == TdsEnums.SQLFEDAUTHINFO);
token == TdsEnums.SQLFEDAUTHINFO ||
token == TdsEnums.SQLDEBUG_CMD );
}

// Main parse loop for the top-level tds tokens, calls back into the I*Handler interfaces
Expand Down Expand Up @@ -2040,8 +2041,9 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead
{
case TdsEnums.SQLERROR:
case TdsEnums.SQLINFO:
case TdsEnums.SQLDEBUG_CMD:
{
if (token == TdsEnums.SQLERROR)
if (token == TdsEnums.SQLERROR || token == TdsEnums.SQLDEBUG_CMD) // This the value of 0x60 send fromm TDS Control token
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what the comment is trying to communicate here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the comment since this was a draft PR and I needed to make a change to run the pipelines I did not pay much attention to it.

The specs do not provide any info on that TDS token stream. At first, I did not notice that 0x60 is available in TDSEnums. basically, the issue was raised by PS team. They tried to provide an invalid token(AE token not TDS header token) to decrypt an encrypted column. Server returns 96 instead of error token (0xaa).

Copy link
Contributor

Choose a reason for hiding this comment

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

If we're getting it from the server then either they need to update the spec or stop sending it. I think updating the spec is easier.

{
stateObj.HasReceivedError = true; // Keep track of the fact error token was received - for Done processing.
}
Expand Down Expand Up @@ -3909,7 +3911,8 @@ internal bool TryProcessError(byte token, TdsParserStateObject stateObj, out Sql
}

Debug.Assert(((errorClass >= TdsEnums.MIN_ERROR_CLASS) && token == TdsEnums.SQLERROR) ||
((errorClass < TdsEnums.MIN_ERROR_CLASS) && token == TdsEnums.SQLINFO), "class and token don't match!");
((errorClass < TdsEnums.MIN_ERROR_CLASS) && token == TdsEnums.SQLINFO) ||
((errorClass < TdsEnums.MIN_ERROR_CLASS) && token == TdsEnums.SQLDEBUG_CMD ), "class and token don't match!");

if (!stateObj.TryReadUInt16(out shortLen))
{
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ internal static class TdsEnums
public const byte SQLDONEINPROC = 0xff;
public const byte SQLOFFSET = 0x78;
public const byte SQLORDER = 0xa9;
public const byte SQLDEBUG_CMD = 0x60;
public const byte SQLDEBUG_CMD = 0x60; // This needs more info
Copy link
Contributor

Choose a reason for hiding this comment

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

Who needs to provide the info? the SQL Server team? The spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to provide more info and add it here 😄 . This is just a replacement for more info and triggering pipelines.

public const byte SQLLOGINACK = 0xad;
public const byte SQLFEATUREEXTACK = 0xae; // TDS 7.4 - feature ack
public const byte SQLSESSIONSTATE = 0xe4; // TDS 7.4 - connection resiliency session state
Expand Down