Skip to content

Commit 4a727d0

Browse files
aristotelosaristotelos
committed
Improve capability validation
1 parent c3b3fca commit 4a727d0

File tree

2 files changed

+116
-15
lines changed

2 files changed

+116
-15
lines changed

‎src/FlaUI.WebDriver.UITests/SessionTests.cs‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,20 @@ public void NewSession_AppNotExists_ReturnsError()
4040
Assert.That(newSession, Throws.TypeOf<WebDriverArgumentException>().With.Message.EqualTo("Starting app 'C:\\NotExisting.exe' with arguments '' threw an exception: An error occurred trying to start process 'C:\\NotExisting.exe' with working directory '.'. The system cannot find the file specified."));
4141
}
4242

43+
[TestCase(123)]
44+
[TestCase(false)]
45+
public void NewSession_AppNotAString_Throws(object value)
46+
{
47+
var driverOptions = new FlaUIDriverOptions()
48+
{
49+
PlatformName = "Windows"
50+
};
51+
driverOptions.AddAdditionalOption("appium:app", value);
52+
53+
Assert.That(() => new RemoteWebDriver(WebDriverFixture.WebDriverUrl, driverOptions),
54+
Throws.TypeOf<WebDriverArgumentException>().With.Message.EqualTo("Capability appium:app must be a string"));
55+
}
56+
4357
[Test]
4458
public void NewSession_AppTopLevelWindow_IsSupported()
4559
{
@@ -91,7 +105,7 @@ public void NewSession_AppTopLevelWindowTitleMatch_IsSupported(string match)
91105
}
92106

93107
[Test, Ignore("Sometimes multiple processes are left open")]
94-
public void NewSession_MultipleMatchingAppTopLevelWindowTitleMatch_ReturnsError()
108+
public void NewSession_AppTopLevelWindowTitleMatchMultipleMatching_ReturnsError()
95109
{
96110
using var testAppProcess = new TestAppProcess();
97111
using var testAppProcess1 = new TestAppProcess();
@@ -125,6 +139,33 @@ public void NewSession_AppTopLevelWindowTitleMatchNotFound_ReturnsError()
125139
Assert.That(newSession, Throws.TypeOf<WebDriverArgumentException>().With.Message.EqualTo("Process with main window title matching 'FlaUI Not Existing' could not be found"));
126140
}
127141

142+
[TestCase(123)]
143+
[TestCase(false)]
144+
public void NewSession_AppTopLevelWindowTitleMatchNotAString_Throws(object value)
145+
{
146+
var driverOptions = new FlaUIDriverOptions()
147+
{
148+
PlatformName = "Windows"
149+
};
150+
driverOptions.AddAdditionalOption("appium:appTopLevelWindowTitleMatch", value);
151+
152+
Assert.That(() => new RemoteWebDriver(WebDriverFixture.WebDriverUrl, driverOptions),
153+
Throws.TypeOf<WebDriverArgumentException>().With.Message.EqualTo("Capability appium:appTopLevelWindowTitleMatch must be a string"));
154+
}
155+
156+
[TestCase("(invalid")]
157+
public void NewSession_AppTopLevelWindowTitleMatchInvalidRegex_Throws(string value)
158+
{
159+
var driverOptions = new FlaUIDriverOptions()
160+
{
161+
PlatformName = "Windows"
162+
};
163+
driverOptions.AddAdditionalOption("appium:appTopLevelWindowTitleMatch", value);
164+
165+
Assert.That(() => new RemoteWebDriver(WebDriverFixture.WebDriverUrl, driverOptions),
166+
Throws.TypeOf<WebDriverArgumentException>().With.Message.EqualTo("Capability appium:appTopLevelWindowTitleMatch '(invalid' is not a valid regular expression: Invalid pattern '(invalid' at offset 8. Not enough )'s."));
167+
}
168+
128169
[TestCase("")]
129170
[TestCase("FlaUI")]
130171
public void NewSession_AppTopLevelWindowInvalidFormat_ReturnsError(string appTopLevelWindowString)
@@ -136,6 +177,20 @@ public void NewSession_AppTopLevelWindowInvalidFormat_ReturnsError(string appTop
136177
Assert.That(newSession, Throws.TypeOf<WebDriverArgumentException>().With.Message.EqualTo($"Capability appium:appTopLevelWindow '{appTopLevelWindowString}' is not a valid hexadecimal string"));
137178
}
138179

180+
[TestCase(123)]
181+
[TestCase(false)]
182+
public void NewSession_AppTopLevelWindowNotAString_ReturnsError(object value)
183+
{
184+
var driverOptions = new FlaUIDriverOptions()
185+
{
186+
PlatformName = "Windows"
187+
};
188+
driverOptions.AddAdditionalOption("appium:appTopLevelWindow", value);
189+
190+
Assert.That(() => new RemoteWebDriver(WebDriverFixture.WebDriverUrl, driverOptions),
191+
Throws.TypeOf<WebDriverArgumentException>().With.Message.EqualTo("Capability appium:appTopLevelWindow must be a string"));
192+
}
193+
139194
[Test]
140195
public void GetTitle_Default_IsSupported()
141196
{
@@ -197,5 +252,17 @@ public void NewCommandTimeout_NotExpired_DoesNotEndSession()
197252

198253
Assert.That(() => driver.Title, Throws.Nothing);
199254
}
255+
256+
[TestCase("123")]
257+
[TestCase(false)]
258+
[TestCase("not a number")]
259+
public void NewCommandTimeout_InvalidValue_Throws(object value)
260+
{
261+
var driverOptions = FlaUIDriverOptions.TestApp();
262+
driverOptions.AddAdditionalOption("appium:newCommandTimeout", value);
263+
264+
Assert.That(() => new RemoteWebDriver(WebDriverFixture.WebDriverUrl, driverOptions),
265+
Throws.TypeOf<WebDriverArgumentException>().With.Message.EqualTo("Capability appium:newCommandTimeout must be a number"));
266+
}
200267
}
201268
}

‎src/FlaUI.WebDriver/Controllers/SessionController.cs‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Diagnostics;
7+
using System.Diagnostics.CodeAnalysis;
78
using System.Linq;
89
using System.Text.Json;
910
using System.Text.RegularExpressions;
@@ -42,25 +43,24 @@ public async Task<ActionResult> CreateNewSession([FromBody] CreateSessionRequest
4243
Message = "Required capabilities did not match. Capability `platformName` with value `windows` is required"
4344
});
4445
}
45-
if (capabilities.TryGetValue("appium:app", out var appPath))
46+
if (TryGetStringCapability(capabilities, "appium:app", out var appPath))
4647
{
47-
if (appPath.GetString() == "Root")
48+
if (appPath == "Root")
4849
{
4950
app = null;
5051
}
5152
else
52-
{
53-
bool hasArguments = capabilities.TryGetValue("appium:appArguments", out var appArgumentsValue);
54-
var appArguments = hasArguments ? appArgumentsValue.GetString()! : "";
53+
{
54+
TryGetStringCapability(capabilities, "appium:appArguments", out var appArguments);
5555
try
5656
{
57-
if (appPath.GetString()!.EndsWith("!App"))
57+
if (appPath.EndsWith("!App"))
5858
{
59-
app = Core.Application.LaunchStoreApp(appPath.GetString()!, appArguments);
59+
app = Core.Application.LaunchStoreApp(appPath, appArguments);
6060
}
6161
else
6262
{
63-
var processStartInfo = new ProcessStartInfo(appPath.GetString()!, appArguments);
63+
var processStartInfo = new ProcessStartInfo(appPath, appArguments ?? "");
6464
app = Core.Application.Launch(processStartInfo);
6565
}
6666
}
@@ -70,24 +70,24 @@ public async Task<ActionResult> CreateNewSession([FromBody] CreateSessionRequest
7070
}
7171
}
7272
}
73-
else if(capabilities.TryGetValue("appium:appTopLevelWindow", out var appTopLevelWindowString))
73+
else if (TryGetStringCapability(capabilities, "appium:appTopLevelWindow", out var appTopLevelWindowString))
7474
{
75-
Process process = GetProcessByMainWindowHandle(appTopLevelWindowString.GetString()!);
75+
Process process = GetProcessByMainWindowHandle(appTopLevelWindowString);
7676
app = Core.Application.Attach(process);
7777
}
78-
else if (capabilities.TryGetValue("appium:appTopLevelWindowTitleMatch", out var appTopLevelWindowTitleMatch))
78+
else if (TryGetStringCapability(capabilities, "appium:appTopLevelWindowTitleMatch", out var appTopLevelWindowTitleMatch))
7979
{
80-
Process? process = GetProcessByMainWindowTitle(appTopLevelWindowTitleMatch.GetString()!);
80+
Process? process = GetProcessByMainWindowTitle(appTopLevelWindowTitleMatch);
8181
app = Core.Application.Attach(process);
8282
}
8383
else
8484
{
8585
throw WebDriverResponseException.InvalidArgument("One of appium:app, appium:appTopLevelWindow or appium:appTopLevelWindowTitleMatch must be passed as a capability");
8686
}
8787
var session = new Session(app);
88-
if(capabilities.TryGetValue("appium:newCommandTimeout", out var newCommandTimeout))
88+
if(TryGetNumberCapability(capabilities, "appium:newCommandTimeout", out var newCommandTimeout))
8989
{
90-
session.NewCommandTimeout = TimeSpan.FromSeconds(newCommandTimeout.GetDouble());
90+
session.NewCommandTimeout = TimeSpan.FromSeconds(newCommandTimeout);
9191
}
9292
_sessionRepository.Add(session);
9393
_logger.LogInformation("Created session with ID {SessionId} and capabilities {Capabilities}", session.SessionId, capabilities);
@@ -98,6 +98,40 @@ public async Task<ActionResult> CreateNewSession([FromBody] CreateSessionRequest
9898
}));
9999
}
100100

101+
private static bool TryGetStringCapability(Dictionary<string, JsonElement> capabilities, string key, [MaybeNullWhen(false)] out string value)
102+
{
103+
if(capabilities.TryGetValue(key, out var valueJson))
104+
{
105+
if(valueJson.ValueKind != JsonValueKind.String)
106+
{
107+
throw WebDriverResponseException.InvalidArgument($"Capability {key} must be a string");
108+
}
109+
110+
value = valueJson.GetString();
111+
return value != null;
112+
}
113+
114+
value = null;
115+
return false;
116+
}
117+
118+
private static bool TryGetNumberCapability(Dictionary<string, JsonElement> capabilities, string key, out double value)
119+
{
120+
if (capabilities.TryGetValue(key, out var valueJson))
121+
{
122+
if (valueJson.ValueKind != JsonValueKind.Number)
123+
{
124+
throw WebDriverResponseException.InvalidArgument($"Capability {key} must be a number");
125+
}
126+
127+
value = valueJson.GetDouble();
128+
return true;
129+
}
130+
131+
value = default;
132+
return false;
133+
}
134+
101135
private static Process GetProcessByMainWindowTitle(string appTopLevelWindowTitleMatch)
102136
{
103137
Regex appMainWindowTitleRegex;

0 commit comments

Comments
 (0)