Skip to content

Commit 69fe9df

Browse files
committed
fix: Add retry-safe download and transport fixes
Add tests and changes to ensure download retries are safe and transports are thread-safe. Tests: add RetryAwareDownloadCaptureTransport and NonSeekableStream plus two tests verifying retry uses initial stream position and non-seekable streams do not retry. InternalDownloadDataStream: capture initial stream position and only attempt retries when stream.CanSeek; return a failed FastbootResponse immediately for non-seekable streams on exception. TcpTransport: introduce an _ioLock and synchronize read/write paths; fix message-length read logic to avoid races. UdpTransport: extract common packet exchange logic into ExchangePacketSequence, consolidate SendSinglePacket/Receive handling, and correct payload/collector/output handling and overflow checks. Project: restrict Fastboot project TargetFrameworks to net10.0. These changes fix concurrency/race issues and make download retry behavior deterministic and testable.
1 parent 0e2b592 commit 69fe9df

File tree

5 files changed

+257
-197
lines changed

5 files changed

+257
-197
lines changed

FirmwareKit.Comm.Fastboot.Tests/FastbootProtocolTests.cs

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,96 @@ public long Write(byte[] data, int length)
129129
public void Dispose() { }
130130
}
131131

132+
private sealed class RetryAwareDownloadCaptureTransport : IFastbootTransport
133+
{
134+
private readonly Queue<byte[]> _readQueue = new();
135+
private int _pendingDownloadBytes;
136+
private bool _failedFirstPayloadWrite;
137+
138+
public MemoryStream DownloadPayload { get; } = new();
139+
140+
public RetryAwareDownloadCaptureTransport(bool failFirstPayloadWrite)
141+
{
142+
_failedFirstPayloadWrite = failFirstPayloadWrite;
143+
}
144+
145+
public byte[] Read(int length)
146+
{
147+
if (_readQueue.Count == 0)
148+
{
149+
return Array.Empty<byte>();
150+
}
151+
return _readQueue.Dequeue();
152+
}
153+
154+
public long Write(byte[] data, int length)
155+
{
156+
string command = Encoding.UTF8.GetString(data, 0, length);
157+
if (command.Equals("getvar:has-crc", StringComparison.OrdinalIgnoreCase))
158+
{
159+
_readQueue.Enqueue(Encoding.UTF8.GetBytes("OKAYno"));
160+
return length;
161+
}
162+
163+
if (command.StartsWith("download:", StringComparison.OrdinalIgnoreCase))
164+
{
165+
string hex = command.Substring("download:".Length);
166+
int size = int.Parse(hex, NumberStyles.HexNumber, CultureInfo.InvariantCulture);
167+
_pendingDownloadBytes = size;
168+
_readQueue.Enqueue(Encoding.UTF8.GetBytes($"DATA{size:x8}"));
169+
return length;
170+
}
171+
172+
if (_pendingDownloadBytes > 0)
173+
{
174+
if (_failedFirstPayloadWrite)
175+
{
176+
_failedFirstPayloadWrite = false;
177+
return Math.Max(0, length - 1);
178+
}
179+
180+
DownloadPayload.Write(data, 0, length);
181+
_pendingDownloadBytes -= length;
182+
if (_pendingDownloadBytes <= 0)
183+
{
184+
_readQueue.Enqueue(Encoding.UTF8.GetBytes("OKAY"));
185+
}
186+
return length;
187+
}
188+
189+
_readQueue.Enqueue(Encoding.UTF8.GetBytes("OKAY"));
190+
return length;
191+
}
192+
193+
public void Dispose() { }
194+
}
195+
196+
private sealed class NonSeekableStream : Stream
197+
{
198+
private readonly Stream _inner;
199+
200+
public NonSeekableStream(Stream inner)
201+
{
202+
_inner = inner;
203+
}
204+
205+
public override bool CanRead => _inner.CanRead;
206+
public override bool CanSeek => false;
207+
public override bool CanWrite => _inner.CanWrite;
208+
public override long Length => throw new NotSupportedException();
209+
public override long Position
210+
{
211+
get => throw new NotSupportedException();
212+
set => throw new NotSupportedException();
213+
}
214+
215+
public override void Flush() => _inner.Flush();
216+
public override int Read(byte[] buffer, int offset, int count) => _inner.Read(buffer, offset, count);
217+
public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
218+
public override void SetLength(long value) => _inner.SetLength(value);
219+
public override void Write(byte[] buffer, int offset, int count) => _inner.Write(buffer, offset, count);
220+
}
221+
132222
[Fact]
133223
public void HandleResponse_Success_ReturnsOkay()
134224
{
@@ -302,6 +392,37 @@ public void DownloadDataStream_DataSizeMismatch_Fails()
302392
Assert.DoesNotContain("Short write", response.Response);
303393
}
304394

395+
[Fact]
396+
public void DownloadDataStream_Retry_UsesInitialStreamPosition()
397+
{
398+
var transport = new RetryAwareDownloadCaptureTransport(failFirstPayloadWrite: true);
399+
var util = new FastbootDriver(transport);
400+
401+
byte[] source = new byte[] { 0xAA, 0xBB, 0x01, 0x02, 0x03, 0x04 };
402+
using var stream = new MemoryStream(source);
403+
stream.Position = 2;
404+
405+
var response = util.DownloadData(stream, 4);
406+
407+
Assert.Equal(FastbootState.Success, response.Result);
408+
Assert.Equal(new byte[] { 0x01, 0x02, 0x03, 0x04 }, transport.DownloadPayload.ToArray());
409+
}
410+
411+
[Fact]
412+
public void DownloadDataStream_NonSeekable_DoesNotRetry()
413+
{
414+
var transport = new RetryAwareDownloadCaptureTransport(failFirstPayloadWrite: true);
415+
var util = new FastbootDriver(transport);
416+
417+
using var baseStream = new MemoryStream(new byte[] { 0x01, 0x02, 0x03, 0x04 });
418+
using var nonSeek = new NonSeekableStream(baseStream);
419+
420+
var response = util.DownloadData(nonSeek, 4);
421+
422+
Assert.Equal(FastbootState.Fail, response.Result);
423+
Assert.Contains("non-seekable stream", response.Response);
424+
}
425+
305426
[Fact]
306427
public void DownloadDataBytes_ZeroLength_Fails()
307428
{

FirmwareKit.Comm.Fastboot/Backend/Network/TcpTransport.cs

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ public class TcpTransport : IFastbootBufferedTransport
88
{
99
private const int DefaultIoTimeoutMs = 30000;
1010
private readonly TcpClient _client = new();
11+
private readonly object _ioLock = new();
1112
private readonly byte[] _readLenBuffer = new byte[8];
1213
private readonly byte[] _writeLenBuffer = new byte[8];
1314
private NetworkStream? _stream;
@@ -96,30 +97,36 @@ public int ReadInto(byte[] buffer, int offset, int length)
9697
throw new ArgumentOutOfRangeException(nameof(length));
9798
}
9899

99-
if (_messageBytesLeft == 0)
100+
lock (_ioLock)
100101
{
101-
if (ReadFully(_readLenBuffer, 0, 8) != 8)
102+
if (_messageBytesLeft == 0)
102103
{
103-
throw new Exception("Failed to read message length from TCP stream.");
104+
if (ReadFully(_readLenBuffer, 0, 8) != 8)
105+
{
106+
throw new Exception("Failed to read message length from TCP stream.");
107+
}
108+
_messageBytesLeft = BinaryPrimitives.ReadInt64BigEndian(_readLenBuffer);
104109
}
105-
_messageBytesLeft = BinaryPrimitives.ReadInt64BigEndian(_readLenBuffer);
106-
}
107110

108-
int toRead = (int)Math.Min(length, _messageBytesLeft);
109-
int actuallyRead = ReadFully(buffer, offset, toRead);
110-
_messageBytesLeft -= actuallyRead;
111-
return actuallyRead;
111+
int toRead = (int)Math.Min(length, _messageBytesLeft);
112+
int actuallyRead = ReadFully(buffer, offset, toRead);
113+
_messageBytesLeft -= actuallyRead;
114+
return actuallyRead;
115+
}
112116
}
113117

114118
public long Write(byte[] data, int length)
115119
{
116120
if (_stream == null) throw new InvalidOperationException("Stream not initialized");
117-
BinaryPrimitives.WriteInt64BigEndian(_writeLenBuffer, length);
121+
lock (_ioLock)
122+
{
123+
BinaryPrimitives.WriteInt64BigEndian(_writeLenBuffer, length);
118124

119-
_stream.Write(_writeLenBuffer, 0, 8);
120-
_stream.Write(data, 0, length);
121-
_stream.Flush();
122-
return length;
125+
_stream.Write(_writeLenBuffer, 0, 8);
126+
_stream.Write(data, 0, length);
127+
_stream.Flush();
128+
return length;
129+
}
123130
}
124131

125132
public void Dispose()

0 commit comments

Comments
 (0)