Skip to content

Commit 7b10652

Browse files
rmarinhoCopilot
andcommitted
Address Copilot review round 3: remove null-forgiving, validate port range
- Replace null! with null in tests (no nullable context in test project) - Replace socketSpec! with property pattern (is not { Length: > 0 }) for null flow - Add port range validation (1-65535) in ReversePortAsync and RemoveReversePortAsync Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2dc5885 commit 7b10652

3 files changed

Lines changed: 10 additions & 6 deletions

File tree

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbPortSpec.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,9 @@ public record AdbPortSpec (AdbProtocol Protocol, int Port)
2727
/// </summary>
2828
public static AdbPortSpec? TryParse (string? socketSpec)
2929
{
30-
if (string.IsNullOrWhiteSpace (socketSpec))
30+
if (socketSpec is not { Length: > 0 } value || string.IsNullOrWhiteSpace (value))
3131
return null;
3232

33-
// socketSpec is guaranteed non-null after IsNullOrWhiteSpace check
34-
var value = socketSpec!;
3533
var colonIndex = value.IndexOf (':');
3634
if (colonIndex <= 0 || colonIndex >= value.Length - 1)
3735
return null;

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,10 @@ public virtual async Task ReversePortAsync (string serial, AdbPortSpec remote, A
259259
throw new ArgumentNullException (nameof (remote));
260260
if (local is null)
261261
throw new ArgumentNullException (nameof (local));
262+
if (remote.Port <= 0 || remote.Port > 65535)
263+
throw new ArgumentOutOfRangeException (nameof (remote), remote.Port, "Port must be between 1 and 65535.");
264+
if (local.Port <= 0 || local.Port > 65535)
265+
throw new ArgumentOutOfRangeException (nameof (local), local.Port, "Port must be between 1 and 65535.");
262266

263267
var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "reverse", remote.ToSocketSpec (), local.ToSocketSpec ());
264268
using var stderr = new StringWriter ();
@@ -279,6 +283,8 @@ public virtual async Task RemoveReversePortAsync (string serial, AdbPortSpec rem
279283
throw new ArgumentException ("Serial must not be empty.", nameof (serial));
280284
if (remote is null)
281285
throw new ArgumentNullException (nameof (remote));
286+
if (remote.Port <= 0 || remote.Port > 65535)
287+
throw new ArgumentOutOfRangeException (nameof (remote), remote.Port, "Port must be between 1 and 65535.");
282288

283289
var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "reverse", "--remove", remote.ToSocketSpec ());
284290
using var stderr = new StringWriter ();

tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,15 +1021,15 @@ public void ReversePortAsync_NullRemote_ThrowsArgumentNull ()
10211021
{
10221022
var runner = new AdbRunner ("/fake/sdk/platform-tools/adb");
10231023
Assert.ThrowsAsync<System.ArgumentNullException> (
1024-
async () => await runner.ReversePortAsync ("emulator-5554", (AdbPortSpec) null!, new AdbPortSpec (AdbProtocol.Tcp, 5000)));
1024+
async () => await runner.ReversePortAsync ("emulator-5554", (AdbPortSpec) null, new AdbPortSpec (AdbProtocol.Tcp, 5000)));
10251025
}
10261026

10271027
[Test]
10281028
public void ReversePortAsync_NullLocal_ThrowsArgumentNull ()
10291029
{
10301030
var runner = new AdbRunner ("/fake/sdk/platform-tools/adb");
10311031
Assert.ThrowsAsync<System.ArgumentNullException> (
1032-
async () => await runner.ReversePortAsync ("emulator-5554", new AdbPortSpec (AdbProtocol.Tcp, 5000), (AdbPortSpec) null!));
1032+
async () => await runner.ReversePortAsync ("emulator-5554", new AdbPortSpec (AdbProtocol.Tcp, 5000), (AdbPortSpec) null));
10331033
}
10341034

10351035
// --- RemoveReversePortAsync parameter validation tests ---
@@ -1047,7 +1047,7 @@ public void RemoveReversePortAsync_NullRemote_ThrowsArgumentNull ()
10471047
{
10481048
var runner = new AdbRunner ("/fake/sdk/platform-tools/adb");
10491049
Assert.ThrowsAsync<System.ArgumentNullException> (
1050-
async () => await runner.RemoveReversePortAsync ("emulator-5554", (AdbPortSpec) null!));
1050+
async () => await runner.RemoveReversePortAsync ("emulator-5554", (AdbPortSpec) null));
10511051
}
10521052

10531053
// --- RemoveAllReversePortsAsync parameter validation tests ---

0 commit comments

Comments
 (0)