Browse Source

Fix TryReadPlpBytes throws ArgumentException (#3470)

* add clearing of data sizes when a multi-part result is returned

* remove asserts detecting overwrite of snapshot storage which are now no longer accurate

* remove empty statement block

---------

Co-authored-by: Wraith2 <Wraith2@gmaill.com>
pull/3482/head
Wraith 2 weeks ago
committed by GitHub
parent
commit
951da4578d
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 11
      src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
  2. 11
      src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
  3. 47
      src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs

11
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs

@ -13405,6 +13405,13 @@ namespace Microsoft.Data.SqlClient
break;
}
}
if (writeDataSizeToSnapshot)
{
stateObj.SetSnapshotStorage(null);
stateObj.ClearSnapshotDataSize();
}
return TdsOperationStatus.Done;
static int IncrementSnapshotDataSize(TdsParserStateObject stateObj, bool resetting, int previousPacketId, int value)
@ -13424,7 +13431,7 @@ namespace Microsoft.Data.SqlClient
current = 0;
}
stateObj.SetSnapshotDataSize(current + value);
stateObj.AddSnapshotDataSize(current + value);
// return new packetid so next time we see this packet we know it isn't new
return currentPacketId;
@ -13432,7 +13439,7 @@ namespace Microsoft.Data.SqlClient
else
{
current = stateObj.GetSnapshotDataSize();
stateObj.SetSnapshotDataSize(current + value);
stateObj.AddSnapshotDataSize(current + value);
return previousPacketId;
}
}

11
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs

@ -13596,6 +13596,13 @@ namespace Microsoft.Data.SqlClient
break;
}
}
if (writeDataSizeToSnapshot)
{
stateObj.SetSnapshotStorage(null);
stateObj.ClearSnapshotDataSize();
}
return TdsOperationStatus.Done;
static int IncrementSnapshotDataSize(TdsParserStateObject stateObj, bool resetting, int previousPacketId, int value)
@ -13615,7 +13622,7 @@ namespace Microsoft.Data.SqlClient
current = 0;
}
stateObj.SetSnapshotDataSize(current + value);
stateObj.AddSnapshotDataSize(current + value);
// return new packetid so next time we see this packet we know it isn't new
return currentPacketId;
@ -13623,7 +13630,7 @@ namespace Microsoft.Data.SqlClient
else
{
current = stateObj.GetSnapshotDataSize();
stateObj.SetSnapshotDataSize(current + value);
stateObj.AddSnapshotDataSize(current + value);
return previousPacketId;
}
}

47
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs

@ -1512,7 +1512,7 @@ namespace Microsoft.Data.SqlClient
if (writeDataSizeToSnapshot)
{
SetSnapshotDataSize(bytesToRead);
AddSnapshotDataSize(bytesToRead);
}
AssertValidState();
@ -2058,7 +2058,7 @@ namespace Microsoft.Data.SqlClient
// bool firstchunk = false;
bool isNull = false;
Debug.Assert(_longlenleft == 0, "Out of synch length read request");
Debug.Assert(_longlenleft == 0, "Out of sync length read request");
if (_longlen == 0)
{
// First chunk is being read. Find out what type of chunk it is
@ -2139,6 +2139,7 @@ namespace Microsoft.Data.SqlClient
}
return TryReadPlpBytes(ref buff, offset, len, out totalBytesRead, canContinue, canContinue, compatibilityMode);
}
// Reads the requested number of bytes from a plp data stream, or the entire data if
// requested length is -1 or larger than the actual length of data. First call to this method
// should be preceeded by a call to ReadPlpLength or ReadDataLength.
@ -2254,14 +2255,14 @@ namespace Microsoft.Data.SqlClient
SetSnapshotStorage(buff);
if (writeDataSizeToSnapshot)
{
SetSnapshotDataSize(bytesRead);
AddSnapshotDataSize(bytesRead);
}
}
return result;
}
if (writeDataSizeToSnapshot && canContinue)
{
SetSnapshotDataSize(bytesRead);
AddSnapshotDataSize(bytesRead);
}
if (_longlenleft == 0)
@ -2279,10 +2280,7 @@ namespace Microsoft.Data.SqlClient
else if (canContinue && result == TdsOperationStatus.NeedMoreData)
{
SetSnapshotStorage(buff);
if (writeDataSizeToSnapshot)
{
SetSnapshotDataSize(bytesRead);
}
// data bytes read from the current packet must be 0 here so do not save the snapshot data size
}
return result;
}
@ -2296,6 +2294,12 @@ namespace Microsoft.Data.SqlClient
break;
}
}
if (canContinue)
{
SetSnapshotStorage(null);
ClearSnapshotDataSize();
}
return TdsOperationStatus.Done;
}
@ -3531,9 +3535,6 @@ namespace Microsoft.Data.SqlClient
{
StateSnapshot snapshot = _snapshot;
_snapshot = null;
// TODO(GH-3385): Not sure what this is trying to assert, but it
// currently fails the DataReader tests.
// Debug.Assert(snapshot._storage == null);
snapshot.Clear();
Interlocked.CompareExchange(ref _cachedSnapshot, snapshot, null);
}
@ -3591,9 +3592,6 @@ namespace Microsoft.Data.SqlClient
internal void SetSnapshotStorage(object buffer)
{
Debug.Assert(_snapshot != null, "should not access snapshot accessor functions without first checking that the snapshot is available");
// TODO(GH-3385): Not sure what this is trying to assert, but it
// currently fails the DataReader tests.
// Debug.Assert(_snapshot._storage == null, "should not overwrite snapshot stored buffer");
if (_snapshot != null)
{
_snapshot._storage = buffer;
@ -3605,12 +3603,18 @@ namespace Microsoft.Data.SqlClient
/// countOfBytesCopiedFromCurrentPacket to be calculated
/// </summary>
/// <param name="countOfBytesCopiedFromCurrentPacket"></param>
internal void SetSnapshotDataSize(int countOfBytesCopiedFromCurrentPacket)
internal void AddSnapshotDataSize(int countOfBytesCopiedFromCurrentPacket)
{
Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "_snapshot must exist to store packet data size");
_snapshot.SetPacketDataSize(countOfBytesCopiedFromCurrentPacket);
}
internal void ClearSnapshotDataSize()
{
Debug.Assert(_snapshot != null, "_snapshot must exist to store packet data size");
_snapshot?.ClearPacketDataSize();
}
internal int GetSnapshotTotalSize()
{
Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "_snapshot must exist to read total size");
@ -4307,6 +4311,16 @@ namespace Microsoft.Data.SqlClient
target.RunningDataSize = total + size;
}
internal void ClearPacketDataSize()
{
PacketData current = _firstPacket;
while (current != null)
{
current.RunningDataSize = 0;
current = current.NextPacket;
}
}
internal int GetPacketDataOffset()
{
int offset = 0;
@ -4356,9 +4370,6 @@ namespace Microsoft.Data.SqlClient
private void ClearState()
{
// TODO(GH-3385): Not sure what this is trying to assert, but it
// currently fails the DataReader tests.
// Debug.Assert(_storage == null);
_storage = null;
_replayStateData.Clear(_stateObj);
_continueStateData?.Clear(_stateObj, trackStack: false);

Loading…
Cancel
Save