From 9f984ce7578c93b56e99176edd182140a9b888ca Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Wed, 6 Nov 2024 21:00:23 +0100 Subject: [PATCH] Align comments netfx/netcore --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 136 +++++++++----- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 171 +++++++++--------- 2 files changed, 178 insertions(+), 129 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index 8718aa1879..75f5fc00a4 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -114,7 +114,7 @@ internal void Stop() private SqlInternalTransaction _currentTransaction; private SqlInternalTransaction _pendingTransaction; // pending transaction for 2005 and beyond. - + // SQLHOT 483 // need to hold on to the transaction id if distributed transaction merely rolls back without defecting. private long _retainedTransactionId = SqlInternalTransaction.NullTransactionId; @@ -145,6 +145,7 @@ internal void Stop() private bool _is2022 = false; private byte[][] _sniSpnBuffer = null; + // UNDONE - need to have some for both instances - both command and default??? // SqlStatistics private SqlStatistics _statistics = null; @@ -543,6 +544,7 @@ internal void Connect( _encryptionOption = EncryptionOptions.NOT_SUP; } + // UNDONE - send "" for instance now, need to fix later SqlClientEventSource.Log.TryTraceEvent(" Sending prelogin handshake"); SendPreLoginHandshake(instanceName, encrypt, integratedSecurity, serverCertificateFilename); @@ -647,7 +649,7 @@ internal void RemoveEncryption() ThrowExceptionAndWarning(_physicalStateObj); } - // create a new packet encryption changes the internal packet size + // create a new packet encryption changes the internal packet size Bug# 228403 _physicalStateObj.ClearAllWritePackets(); } @@ -686,6 +688,9 @@ internal TdsParserStateObject CreateSession() internal TdsParserStateObject GetSession(object owner) { TdsParserStateObject session = null; + + // TODO: Ideally, we would not care what we do here -- the session pooler would know whether it should have either one (non-MARS) or many (MARS) sessions. + if (MARSOn) { session = _sessionPool.GetSession(owner); @@ -763,6 +768,9 @@ private void SendPreLoginHandshake( byte[] payload = new byte[(int)PreLoginOptions.NUMOPT * 5 + TdsEnums.MAX_PRELOGIN_PAYLOAD_LENGTH]; int payloadLength = 0; + // UNDONE - need to do some length verification to ensure packet does not + // get too big!!! Not beyond it's max length! + for (int option = (int)PreLoginOptions.VERSION; option < (int)PreLoginOptions.NUMOPT; option++) { int optionDataSize = 0; @@ -939,7 +947,7 @@ private void EnableSsl(uint info, SqlConnectionEncryptOption encrypt, bool integ } } - // create a new packet encryption changes the internal packet size + // create a new packet encryption changes the internal packet size Bug# 228403 _physicalStateObj.ClearAllWritePackets(); } @@ -952,7 +960,8 @@ private PreLoginHandshakeStatus ConsumePreLoginHandshake( bool tlsFirst, string serverCert) { - marsCapable = _fMARS; // Assign default value + // Assign default values + marsCapable = _fMARS; fedAuthRequired = false; bool is2005OrLater = false; Debug.Assert(_physicalStateObj._syncOverAsync, "Should not attempt pends in a synchronous call"); @@ -991,7 +1000,7 @@ private PreLoginHandshakeStatus ConsumePreLoginHandshake( if (payload[0] == 0xaa) { // If the first byte is 0xAA, we are connecting to a 6.5 or earlier server, which - // is not supported. + // is not supported. SQL BU DT 296425 throw SQL.InvalidSQLServerVersionUnknown(); } @@ -1209,7 +1218,7 @@ internal void Deactivate(bool connectionIsDoomed) } if (_physicalStateObj.HasOpenResult) - { // Need to decrement openResultCount for all pending operations. + { // SQL BU DT 383773 - need to decrement openResultCount for all pending operations. _physicalStateObj.DecrementOpenResultCount(); } } @@ -1365,6 +1374,8 @@ internal void ThrowExceptionAndWarning(TdsParserStateObject stateObj, SqlCommand { if ((_state == TdsParserState.OpenNotLoggedIn) && (_connHandler.ConnectionOptions.MultiSubnetFailover || _loginWithFailover) && (temp.Count == 1) && ((temp[0].Number == TdsEnums.TIMEOUT_EXPIRED) || (temp[0].Number == TdsEnums.SNI_WAIT_TIMEOUT))) { + // DevDiv2 Bug 459546: With "MultiSubnetFailover=yes" in the Connection String, SQLClient incorrectly throws a Timeout using shorter time slice (3-4 seconds), not honoring the actual 'Connect Timeout' + // http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/459546 // For Multisubnet Failover we slice the timeout to make reconnecting faster (with the assumption that the server will not failover instantaneously) // However, when timeout occurs we need to not doom the internal connection and also to mark the TdsParser as closed such that the login will be will retried breakConnection = false; @@ -1550,8 +1561,8 @@ internal SqlError ProcessSNIError(TdsParserStateObject stateObj) len -= iColon; /* The error message should come back in the following format: "TCP Provider: MESSAGE TEXT" - If the message is received on a Win9x OS, the error message will not contain MESSAGE TEXT - If we get an error message with no message text, just return the entire message otherwise + Fix Bug 370686, if the message is received on a Win9x OS, the error message will not contain MESSAGE TEXT + per Bug: 269574. If we get a errormessage with no message text, just return the entire message otherwise return just the message text. */ if (len > 0) @@ -2136,7 +2147,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle } else if (error.Class < TdsEnums.FATAL_ERROR_CLASS) { - // Continue results processing for all non-fatal errors (<20) + // VSTFDEVDIV 479643: continue results processing for all non-fatal errors (<20) stateObj.AddError(error); @@ -2148,7 +2159,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle // halt processing. if (dataStream != null) - { + { // Webdata 104560 if (!dataStream.IsInitialized) { runBehavior = RunBehavior.UntilDone; @@ -2191,7 +2202,8 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle dataStream.BrowseModeInfoConsumed = true; } else - { // no dataStream + { + // no dataStream result = stateObj.TrySkipBytes(tokenLength); if (result != TdsOperationStatus.Done) { @@ -2204,8 +2216,8 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle case TdsEnums.SQLDONE: case TdsEnums.SQLDONEPROC: case TdsEnums.SQLDONEINPROC: - { - // RunBehavior can be modified + { + // RunBehavior can be modified - see SQL BU DT 269516 & 290090 result = TryProcessDone(cmdHandler, dataStream, ref runBehavior, stateObj); if (result != TdsOperationStatus.Done) { @@ -2288,6 +2300,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle case TdsEnums.ENV_DEFECTDTC: case TdsEnums.ENV_TRANSACTIONENDED: case TdsEnums.ENV_COMMITTRAN: + // SQLHOT 483 // Must clear the retain id if the server-side transaction ends by anything other // than rollback. _retainedTransactionId = SqlInternalTransaction.NullTransactionId; @@ -2322,6 +2335,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle } else { + // TODO: While not techically necessary at this point, we are not certain whether TransactionEnded indicates Committed, Aborted or Unknown - this is fine for now, but we should investigate. _currentTransaction.Completed(TransactionState.Unknown); } _currentTransaction = null; @@ -2483,6 +2497,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle if (bulkCopyHandler != null) { + // TODO: Consider improving Bulk Copy performance by avoiding boxing. result = TryProcessRow(stateObj._cleanupMetaData, bulkCopyHandler.CreateRowBuffer(), bulkCopyHandler.CreateIndexMap(), stateObj); if (result != TdsOperationStatus.Done) { @@ -2617,22 +2632,25 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle result = stateObj.TryStartNewRow(isNullCompressed: false); if (result != TdsOperationStatus.Done) { + // altrows are not currently null compressed return result; } - // read will call run until dataReady. Must not read any data if ReturnImmediately set + // read will call run until dataReady. Must not read any data if return immediately set if (RunBehavior.ReturnImmediately != (RunBehavior.ReturnImmediately & runBehavior)) { ushort altRowId; result = stateObj.TryReadUInt16(out altRowId); if (result != TdsOperationStatus.Done) { + // get altRowId return result; } result = TrySkipRow(stateObj._cleanupAltMetaDataSetArray.GetAltMetaData(altRowId), stateObj); if (result != TdsOperationStatus.Done) { + // skip altRow return result; } } @@ -2811,6 +2829,8 @@ private TdsOperationStatus TryProcessEnvChange(int tokenLength, TdsParserStateOb break; case TdsEnums.ENV_LOCALEID: + // UNDONE: this LCID may be incorrect for OEM code pages on 7.0 + // need a way to get lcid from code page result = TryReadTwoStringFields(env, stateObj); if (result != TdsOperationStatus.Done) { @@ -2950,6 +2970,7 @@ private TdsOperationStatus TryProcessEnvChange(int tokenLength, TdsParserStateOb result = stateObj.TryReadInt32(out env._newLength); if (result != TdsOperationStatus.Done) { + // new value has 4 byte length return result; } // read new value with 4 byte length @@ -3197,7 +3218,7 @@ private TdsOperationStatus TryProcessDone(SqlCommand cmd, SqlDataReader reader, stateObj.AddError(new SqlError(0, 0, TdsEnums.MIN_ERROR_CLASS, _server, SQLMessage.SevereError(), "", 0, exception: null, batchIndex: cmd?.GetCurrentBatchIndex() ?? -1)); if (reader != null) - { + { // SQL BU DT 269516 if (!reader.IsInitialized) { run = RunBehavior.UntilDone; @@ -3207,13 +3228,13 @@ private TdsOperationStatus TryProcessDone(SqlCommand cmd, SqlDataReader reader, // Similar to above, only with a more severe error. In this case, if we received // the done_srverror, this exception will be added to the collection regardless. - // The server will always break the connection in this case. + // MDAC #93896. Also, per Ashwin, the server will always break the connection in this case. if ((TdsEnums.DONE_SRVERROR == (TdsEnums.DONE_SRVERROR & status)) && (RunBehavior.Clean != (RunBehavior.Clean & run))) { stateObj.AddError(new SqlError(0, 0, TdsEnums.FATAL_ERROR_CLASS, _server, SQLMessage.SevereError(), "", 0, exception: null, batchIndex: cmd?.GetCurrentBatchIndex() ?? -1)); if (reader != null) - { + { // SQL BU DT 269516 if (!reader.IsInitialized) { run = RunBehavior.UntilDone; @@ -3826,7 +3847,7 @@ private TdsOperationStatus TryProcessLoginAck(TdsParserStateObject stateObj, out // 0x07010000 -> 2000 RTM // Notice server response format is different for bwd compat // 0x71000001 -> 2000 SP1 // 0x72xx0002 -> 2005 RTM - + // information provided by S. Ashwin switch (majorMinor) { case TdsEnums.SQL2005_MAJOR << 24 | TdsEnums.SQL2005_RTM_MINOR: // 2005 @@ -4103,7 +4124,8 @@ internal TdsOperationStatus TryProcessError(byte token, TdsParserStateObject sta string server; - // If the server field is not received use the locally cached value. + // MDAC bug #49307 - server sometimes does not send over server field! In those cases + // we will use our locally cached value. if (byteLen == 0) { server = _server; @@ -4353,10 +4375,9 @@ internal TdsOperationStatus TryProcessReturnValue(int length, TdsParserStateObje { return result; } - - // UTF8 collation + if (rec.collation.IsUTF8) - { + { // UTF8 collation rec.encoding = Encoding.UTF8; } else @@ -4611,10 +4632,10 @@ internal int GetCodePage(SqlCollation collation, TdsParserStateObject stateObj) // were removed in Win2k and beyond, however Sql Server still supports them. // In this case we will mask off the sort id (the leading 1). If that fails, // or we have a culture id other than the cases below, we throw an error and - // throw away the rest of the results. + // throw away the rest of the results. For additional info, see MDAC 65963. - // Sometimes GetCultureInfo will return CodePage 0 instead of throwing. - // This should be treated as an error and functionality switches into the following logic. + // SqlHot 50001398: Sometimes GetCultureInfo will return CodePage 0 instead of throwing. + // This should be treated as an error and functionality switches into the following logic. if (!success || codePage == 0) { switch (cultureId) @@ -5191,9 +5212,8 @@ private TdsOperationStatus TryProcessTypeInfo(TdsParserStateObject stateObj, Sql return result; } - // UTF8 collation if (col.collation.IsUTF8) - { + { // UTF8 collation col.encoding = Encoding.UTF8; } else @@ -5348,6 +5368,9 @@ internal TdsOperationStatus TryProcessTableName(int length, TdsParserStateObject MultiPartTableName mpt; while (length > 0) { + // UNDONE: BUG(?) SQL 8.003 returns two tables sometimes + // (select orderid from orders where orderid < ?) + // need to investigate TdsOperationStatus result = TryProcessOneTable(stateObj, ref length, out mpt); if (result != TdsOperationStatus.Done) { @@ -5525,7 +5548,7 @@ private TdsOperationStatus TryProcessColInfo(_SqlMetaDataSet columns, SqlDataRea col.multiPartTableName = reader.TableNames[col.tableNum - 1]; } - // Expressions are readonly + // MDAC 60109: expressions are readonly if (col.IsExpression) { col.Updatability = 0; @@ -7353,6 +7376,7 @@ private byte[] SerializeSqlMoney(SqlMoney value, int length, TdsParserStateObjec private void WriteSqlMoney(SqlMoney value, int length, TdsParserStateObject stateObj) { + // UNDONE: can I use SqlMoney.ToInt64()? int[] bits = decimal.GetBits(value.Value); // this decimal should be scaled by 10000 (regardless of what the incoming decimal was scaled by) @@ -7951,6 +7975,8 @@ private Task WriteEncodingChar(string s, int numChars, int offset, Encoding enco internal int GetEncodingCharLength(string value, int numChars, int charOffset, Encoding encoding) { + // UNDONE: (PERF) this is an expensive way to get the length. Also, aren't we + // UNDONE: (PERF) going through these steps twice when we write out a value? if (string.IsNullOrEmpty(value)) { return 0; @@ -8083,6 +8109,7 @@ internal TdsOperationStatus TryGetTokenLength(byte token, TdsParserStateObject s } else if (0 == (token & 0x0c)) { + // UNDONE: This should be uint return stateObj.TryReadInt32(out tokenLength); } else @@ -8129,6 +8156,7 @@ private void ProcessAttention(TdsParserStateObject stateObj) } catch (Exception e) { + // UNDONE - should not be catching all exceptions!!! if (!ADP.IsCatchableExceptionType(e)) { throw; @@ -8317,6 +8345,7 @@ internal int WriteFedAuthFeatureRequest(FederatedAuthenticationFeatureExtensionD WriteInt(dataLen, _physicalStateObj); _physicalStateObj.WriteByte(options); + // write workflow for FedAuthLibrary.MSAL // write accessToken for FedAuthLibrary.SecurityToken switch (fedAuthFeatureData.libraryType) { @@ -8520,7 +8549,7 @@ private void WriteLoginData(SqlLogin rec, } WriteInt(rec.packetSize, _physicalStateObj); WriteInt(TdsEnums.CLIENT_PROG_VER, _physicalStateObj); - WriteInt(TdsParserStaticMethods.GetCurrentProcessIdForTdsLoginOnly(), _physicalStateObj); + WriteInt(TdsParserStaticMethods.GetCurrentProcessIdForTdsLoginOnly(), _physicalStateObj); //MDAC 84718 WriteInt(0, _physicalStateObj); // connectionID is unused // Log7Flags (DWORD) @@ -8605,7 +8634,7 @@ private void WriteLoginData(SqlLogin rec, WriteShort(rec.hostName.Length, _physicalStateObj); offset += rec.hostName.Length * 2; - // Only send user/password over if not fSSPI... If both user/password and SSPI are in login + // Only send user/password over if not fSSPI or fed auth MSAL... If both user/password and SSPI are in login // rec, only SSPI is used. Confirmed same behavior as in luxor. if (!rec.useSSPI && !(_connHandler._federatedAuthenticationInfoRequested || _connHandler._federatedAuthenticationRequested)) { @@ -8658,6 +8687,8 @@ private void WriteLoginData(SqlLogin rec, WriteShort(rec.database.Length, _physicalStateObj); offset += rec.database.Length * 2; + // UNDONE: NIC address + // previously we declared the array and simply sent it over - byte[] of 0's if (s_nicAddress == null) { s_nicAddress = TdsParserStaticMethods.GetNetworkPhysicalAddressForTdsLoginOnly(); @@ -8743,6 +8774,7 @@ private void WriteLoginData(SqlLogin rec, } catch (Exception e) { + // UNDONE - should not be catching all exceptions!!! if (ADP.IsCatchableExceptionType(e)) { // be sure to wipe out our buffer if we started sending stuff @@ -8918,7 +8950,7 @@ internal SqlDataReader TdsExecuteTransactionManagerRequest( return null; } - // Promote, Commit and Rollback requests for + // SQLBUDT #20010853 - Promote, Commit and Rollback requests for // delegated transactions often happen while there is an open result // set, so we need to handle them by using a different MARS session, // otherwise we'll write on the physical state objects while someone @@ -8992,7 +9024,7 @@ internal SqlDataReader TdsExecuteTransactionManagerRequest( // Only assign the passed in transaction if it is not equal to the current transaction. // And, if it is not equal, the current actually should be null. Anything else // is a unexpected state. The concern here is mainly for the mixed use of - // T-SQL and API transactions. + // T-SQL and API transactions. See SQL BU DT 345300 for full details and repro. // Expected states: // 1) _pendingTransaction = null, _currentTransaction = null, non null transaction @@ -9117,7 +9149,7 @@ internal void FailureCleanup(TdsParserStateObject stateObj, Exception e) SqlClientEventSource.Log.TryTraceEvent(" Exception caught on ExecuteXXX: '{0}'", e); if (stateObj.HasOpenResult) - { // Need to decrement openResultCount if operation failed. + { // SQL BU DT 383773 - need to decrement openResultCount if operation failed. stateObj.DecrementOpenResultCount(); } @@ -9132,6 +9164,7 @@ internal void FailureCleanup(TdsParserStateObject stateObj, Exception e) bool originalThreadHasParserLock = _connHandler.ThreadHasParserLockForClose; try { + // Dev11 Bug 385286 : ExecuteNonQueryAsync hangs when trying to write a parameter which generates ArgumentException and while handling that exception the server disconnects the connection // Need to set this to true such that if we have an error sending\processing the attention, we won't deadlock ourselves _connHandler.ThreadHasParserLockForClose = true; @@ -9161,12 +9194,12 @@ internal Task TdsExecuteSQLBatch(string text, int timeout, SqlNotificationReques throw SQL.ConnectionLockedForBcpEvent(); } - // Promote, Commit and Rollback requests for + // SQLBUDT #20010853 - Promote, Commit and Rollback requests for // delegated transactions often happen while there is an open result // set, so we need to handle them by using a different MARS session, // otherwise we'll write on the physical state objects while someone // else is using it. When we don't have MARS enabled, we need to - // lock the physical state object to synchronize it's use at least + // lock the physical state object to synchronize its use at least // until we increment the open results count. Once it's been // incremented the delegated transaction requests will fail, so they // won't stomp on anything. @@ -9262,6 +9295,7 @@ internal Task TdsExecuteSQLBatch(string text, int timeout, SqlNotificationReques } catch (Exception e) { + // UNDONE - should not be catching all exceptions!!! if (!ADP.IsCatchableExceptionType(e)) { throw; @@ -9296,7 +9330,7 @@ internal Task TdsExecuteRPC(SqlCommand cmd, IList<_SqlRPC> rpcArray, int timeout _SqlRPC rpcext = null; int tempLen; - // Promote, Commit and Rollback requests for + // SQLBUDT #20010853 - Promote, Commit and Rollback requests for // delegated transactions often happen while there is an open result // set, so we need to handle them by using a different MARS session, // otherwise we'll write on the physical state objects while someone @@ -9412,7 +9446,7 @@ internal Task TdsExecuteRPC(SqlCommand cmd, IList<_SqlRPC> rpcArray, int timeout throw SQL.ParameterDirectionInvalidForOptimizedBinding(param.ParameterName); } - // Validate parameters are not variable length without size and with null value. + // Validate parameters are not variable length without size and with null value. MDAC 66522 param.Validate(i, isCommandProc); // type (parameter record stores the MetaType class which is a helper that encapsulates all the type information we need here) @@ -9765,7 +9799,7 @@ private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParamet maxsize = (size > codePageByteSize) ? size : codePageByteSize; if (maxsize == 0) { - // 2005 doesn't like 0 as MaxSize. Change it to 2 for unicode types + // 2005 doesn't like 0 as MaxSize. Change it to 2 for unicode types (SQL9 - 682322) if (mt.IsNCharType) maxsize = 2; else @@ -9858,6 +9892,7 @@ private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParamet WriteUDTMetaData(value, names[0], names[1], names[2], stateObj); + // UNDONE - re-org to use code below to write value! if (!isNull) { WriteUnsignedLong((ulong)udtVal.Length, stateObj); // PLP length @@ -9873,6 +9908,7 @@ private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParamet WriteUnsignedLong(TdsEnums.SQL_PLP_NULL, stateObj); // PLP Null. } return null; // End of UDT - continue to next parameter. + // UNDONE - need to re-org not to continue at a later point in time. } else if (mt.IsPlp) { @@ -10036,7 +10072,6 @@ private void TDSExecuteRPCParameterSetupFlushCompletion(TdsParserStateObject sta private void WriteEnclaveInfo(TdsParserStateObject stateObj, byte[] enclavePackage) { - //If the server supports enclave computations, write enclave info. if (TceVersionSupported >= TdsEnums.MIN_TCE_VERSION_WITH_ENCLAVE_SUPPORT) { @@ -10250,7 +10285,7 @@ private void WriteSmiTypeInfo(SmiExtendedMetaData metaData, TdsParserStateObject case SqlDbType.Char: stateObj.WriteByte(TdsEnums.SQLBIGCHAR); WriteUnsignedShort(checked((ushort)(metaData.MaxLength)), stateObj); - WriteUnsignedInt(_defaultCollation._info, stateObj); + WriteUnsignedInt(_defaultCollation._info, stateObj); // TODO: Use metadata's collation?? stateObj.WriteByte(_defaultCollation._sortId); break; case SqlDbType.DateTime: @@ -10282,13 +10317,13 @@ private void WriteSmiTypeInfo(SmiExtendedMetaData metaData, TdsParserStateObject case SqlDbType.NChar: stateObj.WriteByte(TdsEnums.SQLNCHAR); WriteUnsignedShort(checked((ushort)(metaData.MaxLength * 2)), stateObj); - WriteUnsignedInt(_defaultCollation._info, stateObj); + WriteUnsignedInt(_defaultCollation._info, stateObj); // TODO: Use metadata's collation?? stateObj.WriteByte(_defaultCollation._sortId); break; case SqlDbType.NText: stateObj.WriteByte(TdsEnums.SQLNVARCHAR); WriteUnsignedShort(unchecked((ushort)SmiMetaData.UnlimitedMaxLengthIndicator), stateObj); - WriteUnsignedInt(_defaultCollation._info, stateObj); + WriteUnsignedInt(_defaultCollation._info, stateObj); // TODO: Use metadata's collation?? stateObj.WriteByte(_defaultCollation._sortId); break; case SqlDbType.NVarChar: @@ -10301,7 +10336,7 @@ private void WriteSmiTypeInfo(SmiExtendedMetaData metaData, TdsParserStateObject { WriteUnsignedShort(checked((ushort)(metaData.MaxLength * 2)), stateObj); } - WriteUnsignedInt(_defaultCollation._info, stateObj); + WriteUnsignedInt(_defaultCollation._info, stateObj); // TODO: Use metadata's collation?? stateObj.WriteByte(_defaultCollation._sortId); break; case SqlDbType.Real: @@ -10327,7 +10362,7 @@ private void WriteSmiTypeInfo(SmiExtendedMetaData metaData, TdsParserStateObject case SqlDbType.Text: stateObj.WriteByte(TdsEnums.SQLBIGVARCHAR); WriteUnsignedShort(unchecked((ushort)SmiMetaData.UnlimitedMaxLengthIndicator), stateObj); - WriteUnsignedInt(_defaultCollation._info, stateObj); + WriteUnsignedInt(_defaultCollation._info, stateObj); // TODO: Use metadata's collation?? stateObj.WriteByte(_defaultCollation._sortId); break; case SqlDbType.Timestamp: @@ -10345,7 +10380,7 @@ private void WriteSmiTypeInfo(SmiExtendedMetaData metaData, TdsParserStateObject case SqlDbType.VarChar: stateObj.WriteByte(TdsEnums.SQLBIGVARCHAR); WriteUnsignedShort(unchecked((ushort)metaData.MaxLength), stateObj); - WriteUnsignedInt(_defaultCollation._info, stateObj); + WriteUnsignedInt(_defaultCollation._info, stateObj); // TODO: Use metadata's collation?? stateObj.WriteByte(_defaultCollation._sortId); break; case SqlDbType.Variant: @@ -10749,6 +10784,10 @@ internal void WriteBulkCopyMetaData(_SqlMetaDataSet metadataCollection, int coun WriteShort(flags, stateObj); // write the flags + // todo: + // for xml WriteTokenLength results in a no-op + // discuss ... + // xml datatype does not have token length in its metadata. So it should be a noop. switch (md.type) { @@ -10759,6 +10798,7 @@ internal void WriteBulkCopyMetaData(_SqlMetaDataSet metadataCollection, int coun stateObj.WriteByte(md.scale); break; case SqlDbType.Xml: + // TODO: This doesn't look right. Needs fixing. stateObj.WriteByteArray(s_xmlMetadataSubstituteSequence, s_xmlMetadataSubstituteSequence.Length, 0); break; case SqlDbType.Udt: @@ -11142,7 +11182,6 @@ private void WriteMarsHeaderData(TdsParserStateObject stateObj, SqlInternalTrans { // Function to send over additional payload header data for 2005 and beyond only. // We may need to update the mars header length if mars header is changed in the future - WriteShort(TdsEnums.HEADERTYPE_MARS, stateObj); if (transaction != null && SqlInternalTransaction.NullTransactionId != transaction.TransactionId) @@ -11196,7 +11235,7 @@ private int GetNotificationHeaderSize(SqlNotificationRequest notificationRequest // SSBDeployment Length (ushort) // SSBDeployment UnicodeStream (unicode text) // Timeout (uint) -- optional - // Don't send timeout value if it is 0 + // WEBDATA 102263: Don't send timeout value if it is 0 int headerLength = 4 + 2 + 2 + (callbackId.Length * 2) + 2 + (service.Length * 2); if (timeout > 0) @@ -11241,6 +11280,7 @@ private void WriteQueryNotificationHeaderData(SqlNotificationRequest notificatio WriteInt(timeout, stateObj); } + // Write the trace header data, not including the trace header length private void WriteTraceHeaderData(TdsParserStateObject stateObj) { Debug.Assert(this.IncludeTraceHeader, "WriteTraceHeaderData can only be called on a 2012 or higher version server and bid trace with the control bit are on"); @@ -11334,6 +11374,8 @@ private void WriteTokenLength(byte token, int length, TdsParserStateObject state if (0 != (token & 0x80)) tokenLength = 2; else if (0 == (token & 0x0c)) + + // UNDONE: This should be uint tokenLength = 4; else tokenLength = 1; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 4ccaf1ca18..4a00de059f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -718,7 +718,6 @@ internal void Connect(ServerInfo serverInfo, _physicalStateObj.SniContext = SniContext.Snix_PreLogin; SqlClientEventSource.Log.TryTraceEvent(" Consuming prelogin handshake"); - PreLoginHandshakeStatus status = ConsumePreLoginHandshake( authType, encrypt, @@ -871,6 +870,7 @@ internal void RemoveEncryption() _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); ThrowExceptionAndWarning(_physicalStateObj); } + // create a new packet encryption changes the internal packet size Bug# 228403 try { } // EmptyTry/Finally to avoid FXCop violation @@ -1277,9 +1277,7 @@ private PreLoginHandshakeStatus ConsumePreLoginHandshake( // Assign default values marsCapable = _fMARS; fedAuthRequired = false; - bool is2005OrLater = false; - Debug.Assert(_physicalStateObj._syncOverAsync, "Should not attempt pends in a synchronous call"); TdsOperationStatus result = _physicalStateObj.TryReadNetworkPacket(); if (result != TdsOperationStatus.Done) @@ -1820,18 +1818,18 @@ internal SqlError ProcessSNIError(TdsParserStateObject stateObj) // error.errorMessage is null terminated with garbage beyond that, since fixed length string errorMessage; errorMessage = string.IsNullOrEmpty(sniError.errorMessage) ? string.Empty : sniError.errorMessage; - // Format SNI errors and add Context Information - // - // General syntax is: - // - // (provider:, error: - ) - // - // errorMessage | sniError | - // ------------------------------------------- - // ==null | x | must never happen - // !=null | != 0 | retrieve corresponding errorMessage from resources - // !=null | == 0 | replace text left of errorMessage - // + /* Format SNI errors and add Context Information + * + * General syntax is: + * + * (provider:, error: - ) + * + * errorMessage | sniError | + * ------------------------------------------- + * ==null | x | must never happen + * !=null | != 0 | retrieve corresponding errorMessage from resources + * !=null | == 0 | replace text left of errorMessage + */ Debug.Assert(!ADP.IsEmpty(errorMessage), "Empty error message received from SNI"); @@ -1872,9 +1870,9 @@ internal SqlError ProcessSNIError(TdsParserStateObject stateObj) } else { - // SNI error. Replace the entire message - // - errorMessage = SQL.GetSNIErrorMessage((int)sniError.sniError); + // SNI error. Replace the entire message + // + errorMessage = SQL.GetSNIErrorMessage((int)sniError.sniError); // If its a LocalDB error, then nativeError actually contains a LocalDB-specific error code, not a win32 error code if (sniError.sniError == (int)SNINativeMethodWrapper.SniSpecialErrors.LocalDBErrorCode) @@ -1988,7 +1986,7 @@ internal void CheckResetConnection(TdsParserStateObject stateObj) } // - // Takes a 16 bit short and writes it. + // Takes a 16 bit short and writes it to the returned buffer. // internal byte[] SerializeShort(int v, TdsParserStateObject stateObj) { @@ -2008,6 +2006,9 @@ internal byte[] SerializeShort(int v, TdsParserStateObject stateObj) return bytes; } + // + // Takes a 16 bit short and writes it. + // internal void WriteShort(int v, TdsParserStateObject stateObj) { ReliabilitySection.Assert("unreliable call to WriteShort"); // you need to setup for a thread abort somewhere before you call this method @@ -2485,8 +2486,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle // anyways so we need to consume all errors. This is not the case // if we have already given out a reader. If we have already given out // a reader we need to throw the error but not halt further processing. We used to - // halt processing and that was a bug preventing the user from - // processing subsequent results. + // halt processing. if (dataStream != null) { // Webdata 104560 @@ -2755,7 +2755,6 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle result = stateObj.TryPeekByte(out peekedToken); if (result != TdsOperationStatus.Done) { - // temporarily cache next byte return result; } @@ -2842,7 +2841,6 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle result = TrySkipRow(stateObj._cleanupMetaData, stateObj); if (result != TdsOperationStatus.Done) { - // skip rows return result; } } @@ -2998,7 +2996,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle } default: - Debug.Assert(false, "Unhandled token: " + token.ToString(CultureInfo.InvariantCulture)); + Debug.Fail("Unhandled token: " + token.ToString(CultureInfo.InvariantCulture)); break; } @@ -3196,7 +3194,7 @@ private TdsOperationStatus TryProcessEnvChange(int tokenLength, TdsParserStateOb return result; } - // give the parser the new collation values in case parameters don't specify one + // Give the parser the new collation values in case parameters don't specify one _defaultCollation = env._newCollation; _defaultLCID = env._newCollation.LCID; @@ -3310,13 +3308,14 @@ private TdsOperationStatus TryProcessEnvChange(int tokenLength, TdsParserStateOb // new value has 4 byte length return result; } + // read new value with 4 byte length env._newBinValue = new byte[env._newLength]; result = stateObj.TryReadByteArray(env._newBinValue, env._newLength); if (result != TdsOperationStatus.Done) { - // read new value with 4 byte length return result; } + result = stateObj.TryReadByte(out byteLength); if (result != TdsOperationStatus.Done) { @@ -3587,7 +3586,7 @@ private TdsOperationStatus TryProcessDone(SqlCommand cmd, SqlDataReader reader, // situations where this can occur are: an invalid buffer received from client, login error // and the server refused our connection, and the case where we are trying to log in but // the server has reached its max connection limit. Bottom line, we need to throw general - // error in the cases where we did not receive a error token along with the DONE_ERROR. + // error in the cases where we did not receive an error token along with the DONE_ERROR. if ((TdsEnums.DONE_ERROR == (TdsEnums.DONE_ERROR & status)) && stateObj.ErrorCount == 0 && stateObj.HasReceivedError == false && (RunBehavior.Clean != (RunBehavior.Clean & run))) { @@ -3639,7 +3638,8 @@ private TdsOperationStatus TryProcessDone(SqlCommand cmd, SqlDataReader reader, Debug.Assert(!((sqlTransaction != null && _distributedTransaction != null) || (_userStartedLocalTransaction != null && _distributedTransaction != null)) , "ProcessDone - have both distributed and local transactions not null!"); - */ // WebData 112722 + */ + // WebData 112722 stateObj.DecrementOpenResultCount(); } @@ -4125,7 +4125,9 @@ private TdsOperationStatus TryProcessSessionState(TdsParserStateObject stateObj, if (!recoverable) { checked - { sdata._unrecoverableStatesCount++; } + { + sdata._unrecoverableStatesCount++; + } } } else @@ -4145,7 +4147,9 @@ private TdsOperationStatus TryProcessSessionState(TdsParserStateObject stateObj, else { checked - { sdata._unrecoverableStatesCount++; } + { + sdata._unrecoverableStatesCount++; + } } sv._recoverable = recoverable; } @@ -4158,6 +4162,7 @@ private TdsOperationStatus TryProcessSessionState(TdsParserStateObject stateObj, } } } + if (buffer != null) { result = stateObj.TryReadByteArray(buffer, stateLen); @@ -4241,22 +4246,30 @@ private TdsOperationStatus TryProcessLoginAck(TdsParserStateObject stateObj, out break; case TdsEnums.SQL2005_MAJOR << 24 | TdsEnums.SQL2005_RTM_MINOR: // 2005 if (increment != TdsEnums.SQL2005_INCREMENT) - { throw SQL.InvalidTDSVersion(); } + { + throw SQL.InvalidTDSVersion(); + } _is2005 = true; break; case TdsEnums.SQL2008_MAJOR << 24 | TdsEnums.SQL2008_MINOR: if (increment != TdsEnums.SQL2008_INCREMENT) - { throw SQL.InvalidTDSVersion(); } + { + throw SQL.InvalidTDSVersion(); + } _is2008 = true; break; case TdsEnums.SQL2012_MAJOR << 24 | TdsEnums.SQL2012_MINOR: if (increment != TdsEnums.SQL2012_INCREMENT) - { throw SQL.InvalidTDSVersion(); } + { + throw SQL.InvalidTDSVersion(); + } _is2012 = true; break; case (uint)TdsEnums.TDS8_MAJOR << 24 | TdsEnums.TDS8_MINOR: if (increment != TdsEnums.TDS8_INCREMENT) - { throw SQL.InvalidTDSVersion(); } + { + throw SQL.InvalidTDSVersion(); + } _is2022 = true; break; default: @@ -4608,7 +4621,7 @@ internal TdsOperationStatus TryProcessReturnValue(int length, return result; } } - byte len; // Length of parameter name + byte len; result = stateObj.TryReadByte(out len); if (result != TdsOperationStatus.Done) { @@ -4684,7 +4697,7 @@ internal TdsOperationStatus TryProcessReturnValue(int length, } // read the MaxLen - // For xml datatpyes, there is no tokenLength + // For xml datatypes, there is no tokenLength int tdsLen; if (tdsType == TdsEnums.SQLXMLTYPE) @@ -4839,7 +4852,7 @@ internal TdsOperationStatus TryProcessReturnValue(int length, { int codePage = GetCodePage(rec.collation, stateObj); - // if the column lcid is the same as the default, use the default encoder + // If the column lcid is the same as the default, use the default encoder if (codePage == _defaultCodePage) { rec.codePage = _defaultCodePage; @@ -5086,13 +5099,12 @@ internal int GetCodePage(SqlCollation collation, TdsParserStateObject stateObj) // If we failed, it is quite possible this is because certain culture id's // were removed in Win2k and beyond, however Sql Server still supports them. - // There is a workaround for the culture id's listed below, which is to mask - // off the sort id (the leading 1). If that fails, or we have a culture id - // other than the special cases below, we throw an error and throw away the - // rest of the results. For additional info, see MDAC 65963. + // In this case we will mask off the sort id (the leading 1). If that fails, + // or we have a culture id other than the cases below, we throw an error and + // throw away the rest of the results. For additional info, see MDAC 65963. // SqlHot 50001398: Sometimes GetCultureInfo will return CodePage 0 instead of throwing. - // treat this as an error also, and switch into the special-case logic. + // This should be treated as an error and functionality switches into the following logic. if (!success || codePage == 0) { CultureInfo ci = null; @@ -5119,7 +5131,7 @@ internal int GetCodePage(SqlCollation collation, TdsParserStateObject stateObj) ADP.TraceExceptionWithoutRethrow(e); } break; - case 0x827: // Non-supported Lithuanian code page, map it to supported Lithuanian. + case 0x827: // Mapping Non-supported Lithuanian code page to supported Lithuanian. try { ci = new CultureInfo(0x427); @@ -5224,13 +5236,11 @@ internal void DrainData(TdsParserStateObject stateObj) } - // iib. - // now read the remaining values off the wire for this row + // Read the remaining values off the wire for this row if (stateObj.Parser.TrySkipRow(metadata, sharedState._nextColumnHeaderToRead, stateObj) != TdsOperationStatus.Done) { throw SQL.SynchronousCallMayNotPend(); } - } } Run(RunBehavior.Clean, null, null, null, stateObj); @@ -5303,7 +5313,7 @@ internal TdsOperationStatus TryProcessAltMetaData(int cColumns, TdsParserStateOb while (byCols > 0) { - result = stateObj.TrySkipBytes(2); // ignore ColNum ... + result = stateObj.TrySkipBytes(2); if (result != TdsOperationStatus.Done) { return result; @@ -5925,7 +5935,7 @@ private TdsOperationStatus TryCommonProcessMetaData(TdsParserStateObject stateOb return result; } - // We get too many DONE COUNTs from the server, causing too meany StatementCompleted event firings. + // We get too many DONE COUNTs from the server, causing too many StatementCompleted event firings. // We only need to fire this event when we actually have a meta data stream with 0 or more rows. stateObj.HasReceivedColumnMetadata = true; return TdsOperationStatus.Done; @@ -6344,6 +6354,7 @@ private TdsOperationStatus TryProcessColumnHeaderNoNBC(SqlMetaDataPriv col, TdsP length = 0; return result; } + isNull = IsNull(col.metaType, longlen); length = (isNull ? 0 : longlen); return TdsOperationStatus.Done; @@ -6796,6 +6807,8 @@ internal bool DeserializeUnencryptedValue(SqlBuffer value, byte[] unencryptedByt byte denormalizedScale = md.baseTI.scale; Debug.Assert(false == md.baseTI.isEncrypted, "Double encryption detected"); + //DEVNOTE: When modifying the following routines (for deserialization) please pay attention to + // deserialization code in DecryptWithKey () method and modify it accordingly. switch (tdsType) { // We normalize to allow conversion across data types. All data types below are serialized into a BIGINT. @@ -7898,7 +7911,7 @@ internal Task WriteSqlVariantValue(object value, int length, int offset, TdsPars // note that we also write out the maxlen and actuallen members (4 bytes each) // in addition to the SQLVariant structure // - // Devnote: DataRows are preceeded by Metadata. The Metadata includes the MaxLen value. + // Devnote: DataRows are preceded by Metadata. The Metadata includes the MaxLen value. // Therefore the sql_variant value must not include the MaxLength. This is the major difference // between this method and WriteSqlVariantValue above. // @@ -8053,7 +8066,7 @@ internal Task WriteSqlVariantDataRowValue(object value, TdsParserStateObject sta Debug.Fail("unknown tds type for sqlvariant!"); break; } // switch - // return point for accumualated writes, note: non-accumulated writes returned from their case statements + // return point for accumulated writes, note: non-accumulated writes returned from their case statements return null; } @@ -8938,7 +8951,7 @@ private static int StateValueLength(int dataLen) } if (write) { - WriteInt(8 + initialLength + currentLength, _physicalStateObj); // length of data w/o total length (initil+current+2*sizeof(DWORD)) + WriteInt(8 + initialLength + currentLength, _physicalStateObj); // length of data w/o total length (initial + current + 2 * sizeof(DWORD)) WriteInt(initialLength, _physicalStateObj); WriteIdentifier(reconnectData._initialDatabase, _physicalStateObj); WriteCollation(reconnectData._initialCollation, _physicalStateObj); @@ -9004,7 +9017,7 @@ internal int WriteFedAuthFeatureRequest(FederatedAuthenticationFeatureExtensionD break; case TdsEnums.FedAuthLibrary.SecurityToken: Debug.Assert(fedAuthFeatureData.accessToken != null, "AccessToken should not be null."); - dataLen = 1 + sizeof(int) + fedAuthFeatureData.accessToken.Length; // length of feature data = 1 byte for library and echo, security token length and sizeof(int) for token lengh itself + dataLen = 1 + sizeof(int) + fedAuthFeatureData.accessToken.Length; // length of feature data = 1 byte for library and echo, security token length and sizeof(int) for token length itself break; default: Debug.Fail("Unrecognized library type for fedauth feature extension request"); @@ -9095,7 +9108,7 @@ internal int WriteFedAuthFeatureRequest(FederatedAuthenticationFeatureExtensionD _physicalStateObj.WriteByteArray(fedAuthFeatureData.accessToken, fedAuthFeatureData.accessToken.Length, 0); break; default: - Debug.Assert(false, "Unrecognized FedAuthLibrary type for feature extension request"); + Debug.Fail("Unrecognized FedAuthLibrary type for feature extension request"); break; } } @@ -9166,7 +9179,7 @@ internal int WriteFedAuthFeatureRequest(FederatedAuthenticationFeatureExtensionD internal int WriteUTF8SupportFeatureRequest(bool write /* if false just calculates the length */) { - int len = 5; // 1byte = featureID, 4bytes = featureData length + int len = 5; // 1byte = featureID, 4bytes = featureData length, sizeof(DWORD) if (write) { @@ -9327,13 +9340,13 @@ private void WriteLoginData(SqlLogin rec, // write offset/length pairs - // note that you must always set ibHostName since it indicaters the beginning of the variable length section of the login record + // note that you must always set ibHostName since it indicates the beginning of the variable length section of the login record WriteShort(offset, _physicalStateObj); // host name offset WriteShort(rec.hostName.Length, _physicalStateObj); offset += rec.hostName.Length * 2; // Only send user/password over if not fSSPI or fed auth MSAL... If both user/password and SSPI are in login - // rec, only SSPI is used. Confirmed same bahavior as in luxor. + // rec, only SSPI is used. Confirmed same behavior as in luxor. if (!rec.useSSPI && !(_connHandler._federatedAuthenticationInfoRequested || _connHandler._federatedAuthenticationRequested)) { WriteShort(offset, _physicalStateObj); // userName offset @@ -9476,7 +9489,7 @@ private void WriteLoginData(SqlLogin rec, } ApplyFeatureExData(requestedFeatures, recoverySessionData, fedAuthFeatureExtensionData, useFeatureExt, length, true); - } // try + } catch (Exception e) { // UNDONE - should not be catching all exceptions!!! @@ -9505,7 +9518,7 @@ private int ApplyFeatureExData(TdsEnums.FeatureExtension requestedFeatures, if ((requestedFeatures & TdsEnums.FeatureExtension.SessionRecovery) != 0) { length += WriteSessionRecoveryFeatureRequest(recoverySessionData, write); - }; + } if ((requestedFeatures & TdsEnums.FeatureExtension.FedAuth) != 0) { SqlClientEventSource.Log.TryTraceEvent(" Sending federated authentication feature request & wirte = {0}", write); @@ -9557,7 +9570,7 @@ private int ApplyFeatureExData(TdsEnums.FeatureExtension requestedFeatures, /// /// Send the access token to the server. /// - /// Type encapuslating a Federated Authentication access token. + /// Type encapsulating a Federated Authentication access token. internal void SendFedAuthToken(SqlFedAuthToken fedAuthToken) { Debug.Assert(fedAuthToken != null, "fedAuthToken cannot be null"); @@ -9622,7 +9635,7 @@ internal byte[] GetDTCAddress(int timeout, TdsParserStateObject stateObj) { Debug.Fail("unexpected length (> Int32.MaxValue) returned from dtcReader.GetBytes"); // if we hit this case we'll just return a null address so that the user - // will get a transcaction enlistment error in the upper layers + // will get a transaction enlistment error in the upper layers } #endif } @@ -9663,7 +9676,7 @@ internal SqlDataReader TdsExecuteTransactionManagerRequest( // set, so we need to handle them by using a different MARS session, // otherwise we'll write on the physical state objects while someone // else is using it. When we don't have MARS enabled, we need to - // lock the physical state object to syncronize it's use at least + // lock the physical state object to synchronize its use at least // until we increment the open results count. Once it's been // incremented the delegated transaction requests will fail, so they // won't stomp on anything. @@ -9680,7 +9693,7 @@ internal SqlDataReader TdsExecuteTransactionManagerRequest( bool hadAsyncWrites = _asyncWrite; try { - // Temprarily disable async writes + // Temporarily disable async writes _asyncWrite = false; // This validation step MUST be done after locking the connection to guarantee we don't @@ -9916,7 +9929,7 @@ internal Task TdsExecuteSQLBatch(string text, int timeout, SqlNotificationReques // set, so we need to handle them by using a different MARS session, // otherwise we'll write on the physical state objects while someone // else is using it. When we don't have MARS enabled, we need to - // lock the physical state object to syncronize it's use at least + // lock the physical state object to synchronize its use at least // until we increment the open results count. Once it's been // incremented the delegated transaction requests will fail, so they // won't stomp on anything. @@ -10052,7 +10065,7 @@ internal Task TdsExecuteRPC(SqlCommand cmd, IList<_SqlRPC> rpcArray, int timeout // set, so we need to handle them by using a different MARS session, // otherwise we'll write on the physical state objects while someone // else is using it. When we don't have MARS enabled, we need to - // lock the physical state object to syncronize it's use at least + // lock the physical state object to synchronize its use at least // until we increment the open results count. Once it's been // incremented the delegated transaction requests will fail, so they // won't stomp on anything. @@ -10133,8 +10146,6 @@ internal Task TdsExecuteRPC(SqlCommand cmd, IList<_SqlRPC> rpcArray, int timeout for (int i = (ii == startRpc) ? startParam : 0; i < parametersLength; i++) { - // Debug.WriteLine("i: " + i.ToString(CultureInfo.InvariantCulture)); - // parameters can be unnamed byte options = 0; SqlParameter param = rpcext.GetParameterByIndex(i, out options); // Since we are reusing the parameters array, we cannot rely on length to indicate no of parameters. @@ -10346,7 +10357,6 @@ internal Task TdsExecuteRPC(SqlCommand cmd, IList<_SqlRPC> rpcArray, int timeout Debug.Assert(!isSqlVal || !isParameterEncrypted, "isParameterEncrypted can be true only if isSqlVal is false."); - // // fixup the types by using the NullableType property of the MetaType class // // following rules should be followed based on feedback from the M-SQL team @@ -10359,7 +10369,7 @@ internal Task TdsExecuteRPC(SqlCommand cmd, IList<_SqlRPC> rpcArray, int timeout // handle variants here: the SQLVariant writing routine will write the maxlen and actual len columns if (mt.TDSType == TdsEnums.SQLVARIANT) { - // devnote: Do we ever hit this codepath? Yes, when a null value is being writen out via a sql variant + // devnote: Do we ever hit this codepath? Yes, when a null value is being written out via a sql variant // param.GetActualSize is not used WriteSqlVariantValue(isSqlVal ? MetaType.GetComValueFromSqlVariant(value) : value, param.GetActualSize(), param.Offset, stateObj); continue; @@ -10969,7 +10979,7 @@ private void WriteSmiParameter(SqlParameter param, int paramIndex, bool sendDefa } else if (param.Direction == ParameterDirection.Output) { - bool isCLRType = param.ParameterIsSqlType; // We have to forward the TYPE info, we need to know what type we are returning. Once we null the paramater we will no longer be able to distinguish what type were seeing. + bool isCLRType = param.ParameterIsSqlType; // We have to forward the TYPE info, we need to know what type we are returning. Once we null the parameter we will no longer be able to distinguish what type were seeing. param.Value = null; value = null; typeCode = ExtendedClrTypeCode.DBNull; @@ -11310,7 +11320,7 @@ private void WriteTvpOrderUnique(SmiExtendedMetaData metaData, TdsParserStateObj flags = TdsEnums.TVP_ORDERDESC_FLAG; } - // Add unique key flage if appropriate + // Add unique key flag if appropriate if (uniqueKeyProperty[i]) { flags |= TdsEnums.TVP_UNIQUE_FLAG; @@ -11662,7 +11672,8 @@ internal object EncryptColumnValue(object value, SqlMetaDataPriv metadata, strin actualLengthInBytes = (isSqlType) ? ((SqlBinary)value).Length : ((byte[])value).Length; if (metadata.baseTI.length > 0 && actualLengthInBytes > metadata.baseTI.length) - { // see comments agove + { + // see comments above actualLengthInBytes = metadata.baseTI.length; } break; @@ -11806,7 +11817,7 @@ internal Task WriteBulkCopyValue(object value, SqlMetaDataPriv metadata, TdsPars ccb = (isSqlType) ? ((SqlBinary)value).Length : ((byte[])value).Length; break; case TdsEnums.SQLUNIQUEID: - ccb = GUID_SIZE; // that's a constant for guid + ccb = GUID_SIZE; break; case TdsEnums.SQLBIGCHAR: case TdsEnums.SQLBIGVARCHAR: @@ -11965,7 +11976,6 @@ private void WriteMarsHeaderData(TdsParserStateObject stateObj, SqlInternalTrans else { // If no transaction, send over retained transaction descriptor (empty if none retained) - // and always 1 for result count. WriteLong(_retainedTransactionId, stateObj); WriteInt(stateObj.IncrementAndObtainOpenResultCount(null), stateObj); } @@ -12029,7 +12039,7 @@ private void WriteQueryNotificationHeaderData(SqlNotificationRequest notificatio // We may need to update the notification header length if the header is changed in the future - Debug.Assert(notificationRequest != null, "notificaitonRequest is null"); + Debug.Assert(notificationRequest != null, "notificationRequest is null"); string callbackId = notificationRequest.UserData; string service = notificationRequest.Options; @@ -12042,9 +12052,7 @@ private void WriteQueryNotificationHeaderData(SqlNotificationRequest notificatio Debug.Assert(UInt16.MaxValue >= service.Length, "Service length is out of range"); Debug.Assert(-1 <= timeout, "Timeout"); - SqlClientEventSource.Log.TryNotificationTraceEvent(" NotificationRequest: userData: '{0}', options: '{1}', timeout: '{2}'", notificationRequest.UserData, notificationRequest.Options, notificationRequest.Timeout); - WriteShort(TdsEnums.HEADERTYPE_QNOTIFICATION, stateObj); // Query notifications Type WriteShort(callbackId.Length * 2, stateObj); // Length in bytes @@ -12448,14 +12456,14 @@ private Task WriteUnterminatedSqlValue(object value, MetaType type, int actualLe } case TdsEnums.SQLUDT: - Debug.Assert(false, "Called WriteSqlValue on UDT param.Should have already been handled"); + Debug.Fail("Called WriteSqlValue on UDT param.Should have already been handled"); throw SQL.UDTUnexpectedResult(value.GetType().AssemblyQualifiedName); default: Debug.Fail("Unknown TdsType!" + type.NullableType.ToString("x2", (IFormatProvider)null)); break; } // switch - // return point for accumualated writes, note: non-accumulated writes returned from their case statements + // return point for accumulated writes, note: non-accumulated writes returned from their case statements return null; } @@ -12795,7 +12803,6 @@ private async Task WriteXmlFeed(XmlDataFeed feed, TdsParserStateObject stateObj, while (!feed._source.EOF && !writer.IsComplete) { - // We are copying nodes from a reader to a writer. This will cause the // XmlDeclaration to be emitted despite ConformanceLevel.Fragment above. // Therefore, we filter out the XmlDeclaration while copying. @@ -13204,7 +13211,7 @@ private Task WriteUnterminatedValue(object value, MetaType type, byte scale, int Debug.Fail("Unknown TdsType!" + type.NullableType.ToString("x2", (IFormatProvider)null)); break; } // switch - // return point for accumualated writes, note: non-accumulated writes returned from their case statements + // return point for accumulated writes, note: non-accumulated writes returned from their case statements return null; // Debug.WriteLine("value: " + value.ToString(CultureInfo.InvariantCulture)); } @@ -13439,7 +13446,6 @@ private byte[] SerializeUnencryptedValue(object value, MetaType type, byte scale default: throw SQL.UnsupportedDatatypeEncryption(type.TypeName); } // switch - // Debug.WriteLine("value: " + value.ToString(CultureInfo.InvariantCulture)); } // For MAX types, this method can only write everything in one big chunk. If multiple @@ -13827,7 +13833,8 @@ internal TdsOperationStatus TryReadPlpUnicodeChars(ref char[] buff, int offst, i totalCharsRead++; } if (stateObj._longlenleft == 0) - { // Read the next chunk or cleanup state if hit the end + { + // Read the next chunk or cleanup state if hit the end result = stateObj.TryReadPlpLength(false, out _); if (result != TdsOperationStatus.Done) {