diff --git a/.github/workflows/ci-dotnet.yml b/.github/workflows/ci-dotnet.yml index 65debef8c10e4..1e2a2c259382d 100644 --- a/.github/workflows/ci-dotnet.yml +++ b/.github/workflows/ci-dotnet.yml @@ -23,4 +23,5 @@ jobs: java-version: 17 os: windows run: | + fsutil 8dot3name set 0 bazel test //dotnet/test/common:ElementFindingTest-firefox //dotnet/test/common:ElementFindingTest-chrome --pin_browsers=true diff --git a/.github/workflows/ci-java.yml b/.github/workflows/ci-java.yml index 569d68d562440..1ebd054c05089 100644 --- a/.github/workflows/ci-java.yml +++ b/.github/workflows/ci-java.yml @@ -22,6 +22,7 @@ jobs: # https://github.com/bazelbuild/rules_jvm_external/issues/1046 java-version: 17 run: | + fsutil 8dot3name set 0 bazel test --flaky_test_attempts 3 //java/test/org/openqa/selenium/chrome:ChromeDriverFunctionalTest ` //java/test/org/openqa/selenium/federatedcredentialmanagement:FederatedCredentialManagementTest ` //java/test/org/openqa/selenium/firefox:FirefoxDriverBuilderTest ` diff --git a/MODULE.bazel b/MODULE.bazel index bd8225d29fc95..c1817b08269e8 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -20,7 +20,7 @@ bazel_dep(name = "rules_cc", version = "0.0.9", dev_dependency = True) bazel_dep(name = "rules_dotnet", version = "0.16.0") bazel_dep(name = "rules_java", version = "7.11.1") bazel_dep(name = "rules_jvm_external", version = "6.3") -bazel_dep(name = "rules_nodejs", version = "6.2.0") +bazel_dep(name = "rules_nodejs", version = "6.3.0") bazel_dep(name = "rules_oci", version = "1.7.6") bazel_dep(name = "rules_pkg", version = "0.10.1") bazel_dep(name = "rules_python", version = "0.33.0") @@ -76,7 +76,7 @@ rules_ts_ext.deps( use_repo(rules_ts_ext, "npm_typescript") esbuild = use_extension("@aspect_rules_esbuild//esbuild:extensions.bzl", "esbuild") -esbuild.toolchain(esbuild_version = "0.19.9") +esbuild.toolchain(esbuild_version = "0.23.0") use_repo(esbuild, "esbuild_toolchains") register_toolchains("@esbuild_toolchains//:all") diff --git a/Rakefile b/Rakefile index 5c2ca0fe42ccb..57486c7976f03 100644 --- a/Rakefile +++ b/Rakefile @@ -483,7 +483,6 @@ namespace :node do new_version = updated_version(old_version, arguments[:version], nightly) ['javascript/node/selenium-webdriver/package.json', - 'package-lock.json', 'javascript/node/selenium-webdriver/BUILD.bazel'].each do |file| text = File.read(file).gsub(old_version, new_version) File.open(file, 'w') { |f| f.puts text } @@ -1050,8 +1049,7 @@ namespace :all do 'py/BUILD.bazel', 'py/setup.py', 'rb/lib/selenium/webdriver/version.rb', - 'rb/Gemfile.lock', - 'package-lock.json']) + 'rb/Gemfile.lock']) print 'Do you want to push the committed changes? (Y/n): ' response = $stdin.gets.chomp.downcase @@ -1106,7 +1104,6 @@ namespace :all do 'java/version.bzl', 'javascript/node/selenium-webdriver/CHANGES.md', 'javascript/node/selenium-webdriver/package.json', - 'package-lock.json', 'py/docs/source/conf.py', 'py/selenium/__init__.py', 'py/selenium/webdriver/__init__.py', diff --git a/WORKSPACE b/WORKSPACE index eb09ccc8726e0..98cec87d366b7 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -34,8 +34,8 @@ rules_closure_toolchains() http_archive( name = "rules_rust", - integrity = "sha256-JLN47ZcAbx9wEr5Jiib4HduZATGLiDgK7oUi/fvotzU=", - urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.42.1/rules_rust-v0.42.1.tar.gz"], + integrity = "sha256-Zx3bP+Xrz53TTQUeynNS+68z+lO/Ye7Qt1pMNIKeVIA=", + urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.52.2/rules_rust-v0.52.2.tar.gz"], ) load("@rules_rust//rust:repositories.bzl", "rules_rust_dependencies", "rust_register_toolchains") diff --git a/common/extensions/webextensions-selenium-example.crx b/common/extensions/webextensions-selenium-example.crx index 38b38003b7ec3..941114eb446e8 100644 Binary files a/common/extensions/webextensions-selenium-example.crx and b/common/extensions/webextensions-selenium-example.crx differ diff --git a/common/extensions/webextensions-selenium-example/manifest.json b/common/extensions/webextensions-selenium-example/manifest.json index 60f2e5460c500..69e480dc60dda 100644 --- a/common/extensions/webextensions-selenium-example/manifest.json +++ b/common/extensions/webextensions-selenium-example/manifest.json @@ -14,6 +14,14 @@ ] } ], + "permissions": [ + "storage", + "scripting" + ], + "host_permissions": [ + "https://*/*", + "http://*/*" + ], "browser_specific_settings": { "gecko": { "id": "webextensions-selenium-example-v3@example.com" diff --git a/dotnet/src/webdriver/NetworkManager.cs b/dotnet/src/webdriver/NetworkManager.cs index 8d91b72bc27b0..7d3131c7a6114 100644 --- a/dotnet/src/webdriver/NetworkManager.cs +++ b/dotnet/src/webdriver/NetworkManager.cs @@ -45,7 +45,7 @@ public NetworkManager(IWebDriver driver) this.session = new Lazy(() => { IDevTools devToolsDriver = driver as IDevTools; - if (session == null) + if (devToolsDriver == null) { throw new WebDriverException("Driver must implement IDevTools to use these features"); } diff --git a/dotnet/src/webdriver/PrintOptions.cs b/dotnet/src/webdriver/PrintOptions.cs index 2c4c82a12bef3..b5c4959a8270a 100644 --- a/dotnet/src/webdriver/PrintOptions.cs +++ b/dotnet/src/webdriver/PrintOptions.cs @@ -101,19 +101,27 @@ public bool ShrinkToFit } /// - /// Gets the dimensions for each page in the printed document. + /// Gets or sets the dimensions for each page in the printed document. /// public PageSize PageDimensions { get { return pageSize; } + set + { + pageSize = value ?? throw new ArgumentNullException(nameof(value)); + } } /// - /// Gets the margins for each page in the doucment. + /// Gets or sets the margins for each page in the doucment. /// public Margins PageMargins { get { return margins; } + set + { + margins = value ?? throw new ArgumentNullException(nameof(value)); + } } /// diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs index d57b13f278180..4b28f7862d138 100644 --- a/dotnet/src/webdriver/SeleniumManager.cs +++ b/dotnet/src/webdriver/SeleniumManager.cs @@ -25,6 +25,9 @@ using System.Text; using System.Text.Json; using System.Text.Json.Serialization; +using static OpenQA.Selenium.SeleniumManagerResponse; + +#nullable enable namespace OpenQA.Selenium { @@ -36,27 +39,25 @@ public static class SeleniumManager { private static readonly ILogger _logger = Log.GetLogger(typeof(SeleniumManager)); - private static readonly string BinaryFullPath = Environment.GetEnvironmentVariable("SE_MANAGER_PATH"); - private static readonly JsonSerializerOptions _serializerOptions = new() { PropertyNameCaseInsensitive = true, TypeInfoResolver = SeleniumManagerSerializerContext.Default }; - static SeleniumManager() + private static readonly Lazy _lazyBinaryFullPath = new(() => { - - if (BinaryFullPath == null) + string? binaryFullPath = Environment.GetEnvironmentVariable("SE_MANAGER_PATH"); + if (binaryFullPath == null) { var currentDirectory = AppContext.BaseDirectory; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - BinaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "windows", "selenium-manager.exe"); + binaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "windows", "selenium-manager.exe"); } else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) { - BinaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "linux", "selenium-manager"); + binaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "linux", "selenium-manager"); } else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { - BinaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "macos", "selenium-manager"); + binaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "macos", "selenium-manager"); } else { @@ -65,11 +66,13 @@ static SeleniumManager() } } - if (!File.Exists(BinaryFullPath)) + if (!File.Exists(binaryFullPath)) { - throw new WebDriverException($"Unable to locate or obtain Selenium Manager binary at {BinaryFullPath}"); + throw new WebDriverException($"Unable to locate or obtain Selenium Manager binary at {binaryFullPath}"); } - } + + return binaryFullPath; + }); /// /// Determines the location of the browser and driver binaries. @@ -88,7 +91,7 @@ public static Dictionary BinaryPaths(string arguments) argsBuilder.Append(" --debug"); } - var smCommandResult = RunCommand(BinaryFullPath, argsBuilder.ToString()); + var smCommandResult = RunCommand(_lazyBinaryFullPath.Value, argsBuilder.ToString()); Dictionary binaryPaths = new() { { "browser_path", smCommandResult.BrowserPath }, @@ -112,10 +115,10 @@ public static Dictionary BinaryPaths(string arguments) /// /// the standard output of the execution. /// - private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName, string arguments) + private static ResultResponse RunCommand(string fileName, string arguments) { Process process = new Process(); - process.StartInfo.FileName = BinaryFullPath; + process.StartInfo.FileName = _lazyBinaryFullPath.Value; process.StartInfo.Arguments = arguments; process.StartInfo.UseShellExecute = false; process.StartInfo.CreateNoWindow = true; @@ -183,7 +186,7 @@ private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName try { - jsonResponse = JsonSerializer.Deserialize(output, _serializerOptions); + jsonResponse = JsonSerializer.Deserialize(output, _serializerOptions)!; } catch (Exception ex) { @@ -222,32 +225,19 @@ private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName } } - internal class SeleniumManagerResponse + internal record SeleniumManagerResponse(IReadOnlyList Logs, ResultResponse Result) { - public IReadOnlyList Logs { get; set; } - - public ResultResponse Result { get; set; } - - public class LogEntryResponse - { - public string Level { get; set; } - - public string Message { get; set; } - } - - public class ResultResponse - { - [JsonPropertyName("driver_path")] - public string DriverPath { get; set; } - - [JsonPropertyName("browser_path")] - public string BrowserPath { get; set; } - } + public record LogEntryResponse(string Level, string Message); + + public record ResultResponse + ( + [property: JsonPropertyName("driver_path")] + string DriverPath, + [property: JsonPropertyName("browser_path")] + string BrowserPath + ); } [JsonSerializable(typeof(SeleniumManagerResponse))] - internal partial class SeleniumManagerSerializerContext : JsonSerializerContext - { - - } + internal partial class SeleniumManagerSerializerContext : JsonSerializerContext; } diff --git a/dotnet/src/webdriver/WebDriver.csproj b/dotnet/src/webdriver/WebDriver.csproj index 2b9c282cb22de..e9acf442d70c1 100644 --- a/dotnet/src/webdriver/WebDriver.csproj +++ b/dotnet/src/webdriver/WebDriver.csproj @@ -106,7 +106,7 @@ - + diff --git a/dotnet/src/webdriver/cdp/README.md b/dotnet/src/webdriver/cdp/README.md index 04fca71638762..f78d0b3d04ba4 100644 --- a/dotnet/src/webdriver/cdp/README.md +++ b/dotnet/src/webdriver/cdp/README.md @@ -52,9 +52,4 @@ perform the following steps, where `` is the major version of the protocol: remove the entry for version `` from the `SupportedDevToolsVersions` dictionary initialization. 3. Remove the version string (`v`) from the `SUPPORTED_DEVTOOLS_VERSIONS` list in [`//dotnet:selenium-dotnet-version.bzl`](https://github.com/SeleniumHQ/selenium/blob/trunk/dotnet/selenium-dotnet-version.bzl). -4. In [`//dotnet/src/webdriver:WebDriver.csproj.prebuild.cmd`](https://github.com/SeleniumHQ/selenium/blob/trunk/dotnet/src/webdriver/WebDriver.csproj.prebuild.cmd), -remove the `if not exist` block for version ``. -5. In [`//dotnet/src/webdriver:WebDriver.csproj.prebuild.sh`](https://github.com/SeleniumHQ/selenium/blob/trunk/dotnet/src/webdriver/WebDriver.csproj.prebuild.sh), -remove the `if-fi` block for version ``. -6. Commit the changes. - +4. Commit the changes. diff --git a/dotnet/test/common/DriverTestFixture.cs b/dotnet/test/common/DriverTestFixture.cs index fd5cc5e693ae1..9fa1757e9b011 100644 --- a/dotnet/test/common/DriverTestFixture.cs +++ b/dotnet/test/common/DriverTestFixture.cs @@ -88,6 +88,8 @@ public abstract class DriverTestFixture public string scrollFrameOutOfViewport = EnvironmentManager.Instance.UrlBuilder.WhereIs("scrolling_tests/frame_with_nested_scrolling_frame_out_of_view.html"); public string scrollFrameInViewport = EnvironmentManager.Instance.UrlBuilder.WhereIs("scrolling_tests/frame_with_nested_scrolling_frame.html"); + public string printPage = EnvironmentManager.Instance.UrlBuilder.WhereIs("printPage.html"); + protected IWebDriver driver; public IWebDriver DriverInstance diff --git a/dotnet/test/common/PrintTest.cs b/dotnet/test/common/PrintTest.cs new file mode 100644 index 0000000000000..1c31882d2db8b --- /dev/null +++ b/dotnet/test/common/PrintTest.cs @@ -0,0 +1,74 @@ +using NUnit.Framework; +using System; + +namespace OpenQA.Selenium +{ + [TestFixture] + public class PrintTest : DriverTestFixture + { + private const string MagicString = "JVBER"; + private ISupportsPrint printer; + + [SetUp] + public void LocalSetUp() + { + Assert.That(driver, Is.InstanceOf(), $"Driver does not support {nameof(ISupportsPrint)}."); + + printer = driver as ISupportsPrint; + + driver.Navigate().GoToUrl(this.printPage); + } + + [Test] + public void CanPrintPage() + { + var pdf = printer.Print(new PrintOptions()); + + Assert.That(pdf.AsBase64EncodedString, Does.Contain(MagicString)); + } + + [Test] + public void CanPrintTwoPages() + { + var options = new PrintOptions(); + + options.AddPageRangeToPrint("1-2"); + + var pdf = printer.Print(options); + + Assert.That(pdf.AsBase64EncodedString, Does.Contain(MagicString)); + } + + [Test] + public void CanPrintWithMostParams() + { + var options = new PrintOptions() + { + Orientation = PrintOrientation.Landscape, + ScaleFactor = 0.5, + PageDimensions = new PrintOptions.PageSize { Width = 200, Height = 100 }, + PageMargins = new PrintOptions.Margins { Top = 1, Bottom = 1, Left = 2, Right = 2 }, + OutputBackgroundImages = true, + ShrinkToFit = false + }; + + options.AddPageRangeToPrint("1-3"); + + var pdf = printer.Print(options); + + Assert.That(pdf.AsBase64EncodedString, Does.Contain(MagicString)); + } + + [Test] + public void PageSizeCannotBeNull() + { + Assert.That(() => new PrintOptions { PageDimensions = null }, Throws.InstanceOf()); + } + + [Test] + public void MarginsCannotBeNull() + { + Assert.That(() => new PrintOptions { PageMargins = null }, Throws.InstanceOf()); + } + } +} diff --git a/java/spotbugs-excludes.xml b/java/spotbugs-excludes.xml index bb0918aea3d02..389ebc78a76f3 100644 --- a/java/spotbugs-excludes.xml +++ b/java/spotbugs-excludes.xml @@ -204,4 +204,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/java/src/org/openqa/selenium/chromium/ChromiumDriver.java b/java/src/org/openqa/selenium/chromium/ChromiumDriver.java index 7a87460878313..f5518714d436f 100644 --- a/java/src/org/openqa/selenium/chromium/ChromiumDriver.java +++ b/java/src/org/openqa/selenium/chromium/ChromiumDriver.java @@ -212,7 +212,8 @@ public ScriptKey pin(String script) { // Create the actual script we're going to use. String scriptToUse = String.format( - "window.seleniumPinnedScript%s = function(){%s}", Math.abs(script.hashCode()), script); + "window.seleniumPinnedScript%s = function(){%s}", + Math.abs((long) script.hashCode()), script); DevTools devTools = getDevTools(); devTools.createSessionIfThereIsNotOne(); diff --git a/java/src/org/openqa/selenium/devtools/v127/v127Network.java b/java/src/org/openqa/selenium/devtools/v127/v127Network.java index d430a582b2358..769c646c40e7b 100644 --- a/java/src/org/openqa/selenium/devtools/v127/v127Network.java +++ b/java/src/org/openqa/selenium/devtools/v127/v127Network.java @@ -127,7 +127,7 @@ public Either createSeMessages(RequestPaused pausedRe } catch (DevToolsException e) { // Redirects don't seem to have bodies int code = pausedReq.getResponseStatusCode().orElse(HTTP_OK); - if (code < 300 && code > 399) { + if (code >= 300 && code <= 399) { LOG.warning("Unable to get body for request id " + pausedReq.getRequestId()); } diff --git a/java/src/org/openqa/selenium/devtools/v128/v128Network.java b/java/src/org/openqa/selenium/devtools/v128/v128Network.java index e333a026dfc64..ccbdcee9426f3 100644 --- a/java/src/org/openqa/selenium/devtools/v128/v128Network.java +++ b/java/src/org/openqa/selenium/devtools/v128/v128Network.java @@ -127,7 +127,7 @@ public Either createSeMessages(RequestPaused pausedRe } catch (DevToolsException e) { // Redirects don't seem to have bodies int code = pausedReq.getResponseStatusCode().orElse(HTTP_OK); - if (code < 300 && code > 399) { + if (code >= 300 && code <= 399) { LOG.warning("Unable to get body for request id " + pausedReq.getRequestId()); } diff --git a/java/src/org/openqa/selenium/devtools/v129/v129Network.java b/java/src/org/openqa/selenium/devtools/v129/v129Network.java index 41c76f9897c46..21651783d74cf 100644 --- a/java/src/org/openqa/selenium/devtools/v129/v129Network.java +++ b/java/src/org/openqa/selenium/devtools/v129/v129Network.java @@ -127,7 +127,7 @@ public Either createSeMessages(RequestPaused pausedRe } catch (DevToolsException e) { // Redirects don't seem to have bodies int code = pausedReq.getResponseStatusCode().orElse(HTTP_OK); - if (code < 300 && code > 399) { + if (code >= 300 && code <= 399) { LOG.warning("Unable to get body for request id " + pausedReq.getRequestId()); } diff --git a/java/src/org/openqa/selenium/devtools/v85/V85Network.java b/java/src/org/openqa/selenium/devtools/v85/V85Network.java index da12dbe42802d..c4bc0df6219ce 100644 --- a/java/src/org/openqa/selenium/devtools/v85/V85Network.java +++ b/java/src/org/openqa/selenium/devtools/v85/V85Network.java @@ -137,7 +137,7 @@ public Either createSeMessages(RequestPaused pausedRe } catch (DevToolsException e) { // Redirects don't seem to have bodies int code = pausedReq.getResponseStatusCode().orElse(HTTP_OK); - if (code < 300 && code > 399) { + if (code >= 300 && code <= 399) { LOG.warning("Unable to get body for request id " + pausedReq.getRequestId()); } diff --git a/java/src/org/openqa/selenium/grid/data/NodeStatus.java b/java/src/org/openqa/selenium/grid/data/NodeStatus.java index 47753585ec989..c9ce38614fcea 100644 --- a/java/src/org/openqa/selenium/grid/data/NodeStatus.java +++ b/java/src/org/openqa/selenium/grid/data/NodeStatus.java @@ -214,6 +214,8 @@ public boolean equals(Object o) { return Objects.equals(this.nodeId, that.nodeId) && Objects.equals(this.externalUri, that.externalUri) && this.maxSessionCount == that.maxSessionCount + && this.sessionTimeout.compareTo(that.sessionTimeout) == 0 + && this.heartbeatPeriod.compareTo(that.heartbeatPeriod) == 0 && Objects.equals(this.slots, that.slots) && Objects.equals(this.availability, that.availability) && Objects.equals(this.version, that.version); @@ -224,7 +226,7 @@ public int hashCode() { return Objects.hash(nodeId, externalUri, maxSessionCount, slots, version); } - private Map toJson() { + public Map toJson() { Map toReturn = new TreeMap<>(); toReturn.put("nodeId", nodeId); toReturn.put("externalUri", externalUri); diff --git a/java/src/org/openqa/selenium/grid/graphql/Grid.java b/java/src/org/openqa/selenium/grid/graphql/Grid.java index da67f9f6e586f..a6eb03cb6e35d 100644 --- a/java/src/org/openqa/selenium/grid/graphql/Grid.java +++ b/java/src/org/openqa/selenium/grid/graphql/Grid.java @@ -97,6 +97,7 @@ public List getNodes() { status.getExternalUri(), status.getAvailability(), status.getMaxSessionCount(), + status.getSessionTimeout(), status.getSlots().size(), stereotypes, sessions, diff --git a/java/src/org/openqa/selenium/grid/graphql/Node.java b/java/src/org/openqa/selenium/grid/graphql/Node.java index 564ef6e8eef95..267789c5a8fd3 100644 --- a/java/src/org/openqa/selenium/grid/graphql/Node.java +++ b/java/src/org/openqa/selenium/grid/graphql/Node.java @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableList; import java.net.URI; +import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -38,6 +39,7 @@ public class Node { private final URI uri; private final Availability status; private final int maxSession; + private final Duration sessionTimeout; private final Map stereotypes; private final Map activeSessions; private final String version; @@ -49,6 +51,7 @@ public Node( URI uri, Availability status, int maxSession, + Duration sessionTimeout, int slotCount, Map stereotypes, Map activeSessions, @@ -63,6 +66,7 @@ public Node( this.activeSessions = Require.nonNull("Active sessions", activeSessions); this.version = Require.nonNull("Grid Node version", version); this.osInfo = Require.nonNull("Grid Node OS info", osInfo); + this.sessionTimeout = Require.positive("Node session timeout", sessionTimeout); } public List getSessions() { @@ -122,6 +126,10 @@ public OsInfo getOsInfo() { return osInfo; } + public Duration getSessionTimeout() { + return sessionTimeout; + } + private org.openqa.selenium.grid.graphql.Session createGraphqlSession( Map.Entry entry) { Session session = entry.getKey(); diff --git a/java/src/org/openqa/selenium/grid/graphql/selenium-grid-schema.graphqls b/java/src/org/openqa/selenium/grid/graphql/selenium-grid-schema.graphqls index 96c5b9e29f6cc..9278611699c88 100644 --- a/java/src/org/openqa/selenium/grid/graphql/selenium-grid-schema.graphqls +++ b/java/src/org/openqa/selenium/grid/graphql/selenium-grid-schema.graphqls @@ -38,6 +38,7 @@ type Node { uri: Uri! status: Status! maxSession: Int! + sessionTimeout: String! slotCount: Int! sessions: [Session]! sessionCount: Int! diff --git a/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java b/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java index 458dfff372cbe..fadcfff4c80fd 100644 --- a/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java +++ b/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java @@ -17,7 +17,13 @@ package org.openqa.selenium.grid.node; +import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; +import static org.openqa.selenium.remote.HttpSessionId.getSessionId; +import static org.openqa.selenium.remote.http.Contents.asJson; + +import com.google.common.collect.ImmutableMap; import org.openqa.selenium.internal.Require; +import org.openqa.selenium.remote.SessionId; import org.openqa.selenium.remote.http.HttpHandler; import org.openqa.selenium.remote.http.HttpRequest; import org.openqa.selenium.remote.http.HttpResponse; @@ -30,8 +36,22 @@ class ForwardWebDriverCommand implements HttpHandler { this.node = Require.nonNull("Node", node); } + public boolean matches(HttpRequest req) { + return getSessionId(req.getUri()) + .map(id -> node.isSessionOwner(new SessionId(id))) + .orElse(false); + } + @Override public HttpResponse execute(HttpRequest req) { - return node.executeWebDriverCommand(req); + if (matches(req)) { + return node.executeWebDriverCommand(req); + } + return new HttpResponse() + .setStatus(HTTP_INTERNAL_ERROR) + .setContent( + asJson( + ImmutableMap.of( + "error", String.format("Session not found in node %s", node.getId())))); } } diff --git a/java/src/org/openqa/selenium/grid/node/Node.java b/java/src/org/openqa/selenium/grid/node/Node.java index 5d54bb3e2981d..09fe7d02ae9f5 100644 --- a/java/src/org/openqa/selenium/grid/node/Node.java +++ b/java/src/org/openqa/selenium/grid/node/Node.java @@ -152,7 +152,7 @@ protected Node( req -> getSessionId(req.getUri()) .map(SessionId::new) - .map(this::isSessionOwner) + .map(sessionId -> this.getSession(sessionId) != null) .orElse(false)) .to(() -> new ForwardWebDriverCommand(this)) .with(spanDecorator("node.forward_command")), diff --git a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java index 2283e8573042d..7304db8a87847 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java @@ -297,7 +297,13 @@ protected LocalNode( heartbeatPeriod.getSeconds(), TimeUnit.SECONDS); - Runtime.getRuntime().addShutdownHook(new Thread(this::stopAllSessions)); + Runtime.getRuntime() + .addShutdownHook( + new Thread( + () -> { + stopAllSessions(); + drain(); + })); new JMXHelper().register(this); } @@ -316,7 +322,6 @@ private void stopTimedOutSession(RemovalNotification not } // Attempt to stop the session slot.stop(); - this.sessionToDownloadsDir.invalidate(id); // Decrement pending sessions if Node is draining if (this.isDraining()) { int done = pendingSessions.decrementAndGet(); @@ -473,8 +478,6 @@ public Either newSession( sessionToDownloadsDir.put(session.getId(), uuidForSessionDownloads); currentSessions.put(session.getId(), slotToUse); - checkSessionCount(); - SessionId sessionId = session.getId(); Capabilities caps = session.getCapabilities(); SESSION_ID.accept(span, sessionId); @@ -513,6 +516,8 @@ public Either newSession( span.addEvent("Unable to create session with the driver", attributeMap); return Either.left(possibleSession.left()); } + } finally { + checkSessionCount(); } } @@ -765,6 +770,10 @@ public HttpResponse uploadFile(HttpRequest req, SessionId id) { public void stop(SessionId id) throws NoSuchSessionException { Require.nonNull("Session ID", id); + if (sessionToDownloadsDir.getIfPresent(id) != null) { + sessionToDownloadsDir.invalidate(id); + } + SessionSlot slot = currentSessions.getIfPresent(id); if (slot == null) { throw new NoSuchSessionException("Cannot find session with id: " + id); diff --git a/java/src/org/openqa/selenium/grid/router/GridStatusHandler.java b/java/src/org/openqa/selenium/grid/router/GridStatusHandler.java index c873ecaeb003d..b1fb021cf59a9 100644 --- a/java/src/org/openqa/selenium/grid/router/GridStatusHandler.java +++ b/java/src/org/openqa/selenium/grid/router/GridStatusHandler.java @@ -141,6 +141,7 @@ public HttpResponse execute(HttpRequest req) { .put("id", node.getNodeId()) .put("uri", node.getExternalUri()) .put("maxSessions", node.getMaxSessionCount()) + .put("sessionTimeout", node.getSessionTimeout().toMillis()) .put("osInfo", node.getOsInfo()) .put("heartbeatPeriod", node.getHeartbeatPeriod().toMillis()) .put("availability", node.getAvailability()) diff --git a/java/src/org/openqa/selenium/net/PortProber.java b/java/src/org/openqa/selenium/net/PortProber.java index 7106007d28f6b..e132157a33f62 100644 --- a/java/src/org/openqa/selenium/net/PortProber.java +++ b/java/src/org/openqa/selenium/net/PortProber.java @@ -119,7 +119,7 @@ private static boolean isFree(String bindHost, int port) { socket.setReuseAddress(true); socket.bind(new InetSocketAddress(bindHost, port)); return true; - } catch (Exception e) { + } catch (IOException | RuntimeException e) { return false; } } diff --git a/java/test/org/openqa/selenium/grid/distributor/local/LocalDistributorTest.java b/java/test/org/openqa/selenium/grid/distributor/local/LocalDistributorTest.java index eaba9689faa80..3ce2fe80d8352 100644 --- a/java/test/org/openqa/selenium/grid/distributor/local/LocalDistributorTest.java +++ b/java/test/org/openqa/selenium/grid/distributor/local/LocalDistributorTest.java @@ -102,6 +102,8 @@ public void setUp() throws URISyntaxException { LocalNode.builder(tracer, bus, uri, uri, registrationSecret) .add(caps, new TestSessionFactory((id, c) -> new Handler(c))) .maximumConcurrentSessions(2) + .sessionTimeout(Duration.ofSeconds(30)) + .heartbeatPeriod(Duration.ofSeconds(5)) .build(); wait = @@ -143,6 +145,8 @@ void testAddNodeToDistributor() { NodeStatus distributorNode = nodes.iterator().next(); assertThat(distributorNode.getNodeId()).isEqualByComparingTo(localNode.getId()); assertThat(distributorNode.getExternalUri()).isEqualTo(uri); + assertThat(distributorNode.getSessionTimeout()).isEqualTo(Duration.ofSeconds(30)); + assertThat(distributorNode.getHeartbeatPeriod()).isEqualTo(Duration.ofSeconds(5)); } @Test diff --git a/java/test/org/openqa/selenium/grid/gridui/OverallGridTest.java b/java/test/org/openqa/selenium/grid/gridui/OverallGridTest.java index 586230b88a402..b508370e27b20 100644 --- a/java/test/org/openqa/selenium/grid/gridui/OverallGridTest.java +++ b/java/test/org/openqa/selenium/grid/gridui/OverallGridTest.java @@ -68,7 +68,7 @@ public void tearDown() { @Test void shouldReportConcurrencyZeroPercentWhenGridIsStartedWithoutLoad() { - driver.get(whereIs(server, "/ui#/sessions")); + driver.get(whereIs(server, "/ui/#/sessions")); WebElement concurrency = wait.until( @@ -79,7 +79,7 @@ void shouldReportConcurrencyZeroPercentWhenGridIsStartedWithoutLoad() { @Test void shouldShowOneNodeRegistered() { - driver.get(whereIs(server, "/ui")); + driver.get(whereIs(server, "/ui/")); List nodeInfoIcons = wait.until( @@ -93,7 +93,7 @@ void shouldIncrementSessionCountWhenSessionStarts() { WebDriver remoteWebDriver = new RemoteWebDriver(server.getUrl(), Browser.detect().getCapabilities()); try { - driver.get(whereIs(server, "/ui#/sessions")); + driver.get(whereIs(server, "/ui/#/sessions")); wait.until(textToBe(By.cssSelector("div[data-testid='session-count']"), "1")); } finally { diff --git a/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java b/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java new file mode 100644 index 0000000000000..8f0df29251870 --- /dev/null +++ b/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java @@ -0,0 +1,80 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.openqa.selenium.grid.node; + +import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.*; +import static org.openqa.selenium.remote.http.Contents.asJson; + +import com.google.common.collect.ImmutableMap; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.grid.data.NodeId; +import org.openqa.selenium.remote.SessionId; +import org.openqa.selenium.remote.http.HttpRequest; +import org.openqa.selenium.remote.http.HttpResponse; + +class ForwardWebDriverCommandTest { + + private Node mockNode; + private ForwardWebDriverCommand command; + + @BeforeEach + void setUp() { + mockNode = mock(Node.class); + when(mockNode.getId()).thenReturn(new NodeId(UUID.randomUUID())); + command = new ForwardWebDriverCommand(mockNode); + } + + @Test + void testExecuteWithValidSessionOwner() { + HttpRequest mockRequest = mock(HttpRequest.class); + when(mockRequest.getUri()).thenReturn("/session/1234"); + + SessionId sessionId = new SessionId("1234"); + when(mockNode.isSessionOwner(sessionId)).thenReturn(true); + + HttpResponse expectedResponse = new HttpResponse(); + when(mockNode.executeWebDriverCommand(mockRequest)).thenReturn(expectedResponse); + + HttpResponse actualResponse = command.execute(mockRequest); + assertEquals(expectedResponse, actualResponse); + } + + @Test + void testExecuteWithInvalidSessionOwner() { + HttpRequest mockRequest = mock(HttpRequest.class); + when(mockRequest.getUri()).thenReturn("/session/5678"); + + SessionId sessionId = new SessionId("5678"); + when(mockNode.isSessionOwner(sessionId)).thenReturn(false); + + HttpResponse actualResponse = command.execute(mockRequest); + HttpResponse expectResponse = + new HttpResponse() + .setStatus(HTTP_INTERNAL_ERROR) + .setContent( + asJson( + ImmutableMap.of( + "error", String.format("Session not found in node %s", mockNode.getId())))); + assertEquals(expectResponse.getStatus(), actualResponse.getStatus()); + assertEquals(expectResponse.getContentEncoding(), actualResponse.getContentEncoding()); + } +} diff --git a/java/test/org/openqa/selenium/grid/node/NodeTest.java b/java/test/org/openqa/selenium/grid/node/NodeTest.java index b1c50ed0dc65a..c2a66d2162c4b 100644 --- a/java/test/org/openqa/selenium/grid/node/NodeTest.java +++ b/java/test/org/openqa/selenium/grid/node/NodeTest.java @@ -23,6 +23,8 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.InstanceOfAssertFactories.MAP; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.openqa.selenium.json.Json.MAP_TYPE; import static org.openqa.selenium.remote.http.Contents.string; import static org.openqa.selenium.remote.http.HttpMethod.DELETE; @@ -102,7 +104,9 @@ class NodeTest { private Tracer tracer; private EventBus bus; private LocalNode local; + private LocalNode local2; private Node node; + private Node node2; private ImmutableCapabilities stereotype; private ImmutableCapabilities caps; private URI uri; @@ -150,6 +154,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { builder = builder.enableManagedDownloads(true).sessionTimeout(Duration.ofSeconds(1)); } local = builder.build(); + local2 = builder.build(); node = new RemoteNode( @@ -160,6 +165,16 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { registrationSecret, local.getSessionTimeout(), ImmutableSet.of(caps)); + + node2 = + new RemoteNode( + tracer, + new PassthroughHttpClient.Factory(local2), + new NodeId(UUID.randomUUID()), + uri, + registrationSecret, + local2.getSessionTimeout(), + ImmutableSet.of(caps)); } @Test @@ -371,13 +386,36 @@ void shouldOnlyRespondToWebDriverCommandsForSessionsTheNodeOwns() { assertThatEither(response).isRight(); Session session = response.right().getSession(); + Either response2 = + node2.newSession(createSessionRequest(caps)); + assertThatEither(response2).isRight(); + Session session2 = response2.right().getSession(); + + // Assert that should respond to commands for sessions Node 1 owns HttpRequest req = new HttpRequest(POST, String.format("/session/%s/url", session.getId())); assertThat(local.matches(req)).isTrue(); assertThat(node.matches(req)).isTrue(); - req = new HttpRequest(POST, String.format("/session/%s/url", UUID.randomUUID())); - assertThat(local.matches(req)).isFalse(); - assertThat(node.matches(req)).isFalse(); + // Assert that should respond to commands for sessions Node 2 owns + HttpRequest req2 = new HttpRequest(POST, String.format("/session/%s/url", session2.getId())); + assertThat(local2.matches(req2)).isTrue(); + assertThat(node2.matches(req2)).isTrue(); + + // Assert that should not respond to commands for sessions Node 1 does not own + NoSuchSessionException exception = + assertThrows(NoSuchSessionException.class, () -> node.execute(req2)); + assertTrue( + exception + .getMessage() + .startsWith(String.format("Cannot find session with id: %s", session2.getId()))); + + // Assert that should not respond to commands for sessions Node 2 does not own + NoSuchSessionException exception2 = + assertThrows(NoSuchSessionException.class, () -> node2.execute(req)); + assertTrue( + exception2 + .getMessage() + .startsWith(String.format("Cannot find session with id: %s", session.getId()))); } @Test diff --git a/java/test/org/openqa/selenium/grid/router/StressTest.java b/java/test/org/openqa/selenium/grid/router/StressTest.java index a2c432229297f..3be76395e4f01 100644 --- a/java/test/org/openqa/selenium/grid/router/StressTest.java +++ b/java/test/org/openqa/selenium/grid/router/StressTest.java @@ -20,6 +20,8 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.MINUTES; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.StringReader; import java.util.LinkedList; @@ -33,7 +35,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.openqa.selenium.By; +import org.openqa.selenium.MutableCapabilities; +import org.openqa.selenium.NoSuchSessionException; import org.openqa.selenium.WebDriver; +import org.openqa.selenium.WebDriverException; import org.openqa.selenium.grid.config.MapConfig; import org.openqa.selenium.grid.config.MemoizedConfig; import org.openqa.selenium.grid.config.TomlConfig; @@ -65,7 +70,14 @@ public void setupServers() { DeploymentTypes.DISTRIBUTED.start( browser.getCapabilities(), new TomlConfig( - new StringReader("[node]\n" + "driver-implementation = " + browser.displayName()))); + new StringReader( + "[node]\n" + + "driver-implementation = " + + browser.displayName() + + "\n" + + "session-timeout = 11" + + "\n" + + "enable-managed-downloads = true"))); tearDowns.add(deployment); server = deployment.getServer(); @@ -106,7 +118,10 @@ void multipleSimultaneousSessions() throws Exception { try { WebDriver driver = RemoteWebDriver.builder() - .oneOf(browser.getCapabilities()) + .oneOf( + browser + .getCapabilities() + .merge(new MutableCapabilities(Map.of("se:downloadsEnabled", true)))) .address(server.getUrl()) .build(); @@ -124,4 +139,44 @@ void multipleSimultaneousSessions() throws Exception { CompletableFuture.allOf(futures).get(4, MINUTES); } + + @Test + void multipleSimultaneousSessionsTimedOut() throws Exception { + assertThat(server.isStarted()).isTrue(); + + CompletableFuture[] futures = new CompletableFuture[10]; + for (int i = 0; i < futures.length; i++) { + CompletableFuture future = new CompletableFuture<>(); + futures[i] = future; + + executor.submit( + () -> { + try { + WebDriver driver = + RemoteWebDriver.builder() + .oneOf(browser.getCapabilities()) + .address(server.getUrl()) + .build(); + driver.get(appServer.getUrl().toString()); + Thread.sleep(11000); + NoSuchSessionException exception = + assertThrows(NoSuchSessionException.class, driver::getTitle); + assertTrue(exception.getMessage().startsWith("Cannot find session with id:")); + WebDriverException webDriverException = + assertThrows( + WebDriverException.class, + () -> ((RemoteWebDriver) driver).getDownloadableFiles()); + assertTrue( + webDriverException + .getMessage() + .startsWith("Cannot find downloads file system for session id:")); + future.complete(true); + } catch (Exception e) { + future.completeExceptionally(e); + } + }); + } + + CompletableFuture.allOf(futures).get(5, MINUTES); + } } diff --git a/javascript/grid-ui/src/components/LiveView/LiveView.tsx b/javascript/grid-ui/src/components/LiveView/LiveView.tsx index b1d4ea1113811..503edbaa5e54d 100644 --- a/javascript/grid-ui/src/components/LiveView/LiveView.tsx +++ b/javascript/grid-ui/src/components/LiveView/LiveView.tsx @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -import React, { useEffect, useState } from 'react' +import React, { useEffect, useState, useImperativeHandle, forwardRef } from 'react' import RFB from '@novnc/novnc/core/rfb' import PasswordDialog from './PasswordDialog' import MuiAlert, { AlertProps } from '@mui/material/Alert' @@ -28,7 +28,7 @@ const Alert = React.forwardRef(function Alert ( return }) -function LiveView (props) { +const LiveView = forwardRef((props, ref) => { let canvas: any = null const [open, setOpen] = useState(false) @@ -49,6 +49,10 @@ function LiveView (props) { setRfb(null) } + useImperativeHandle(ref, () => ({ + disconnect + })) + const connect = () => { disconnect() @@ -109,6 +113,7 @@ function LiveView (props) { } const handlePasswordDialogClose = () => { + disconnect() props.onClose() } @@ -132,6 +137,7 @@ function LiveView (props) { return } setOpenErrorAlert(false) + disconnect() props.onClose() } @@ -176,6 +182,6 @@ function LiveView (props) { ) -} +}) export default LiveView diff --git a/javascript/grid-ui/src/components/RunningSessions/RunningSessions.tsx b/javascript/grid-ui/src/components/RunningSessions/RunningSessions.tsx index fcc666f3457c4..43fd2c1138f83 100644 --- a/javascript/grid-ui/src/components/RunningSessions/RunningSessions.tsx +++ b/javascript/grid-ui/src/components/RunningSessions/RunningSessions.tsx @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -import React, { useState } from 'react' +import React, { useState, useRef } from 'react' import Table from '@mui/material/Table' import TableBody from '@mui/material/TableBody' import TableCell from '@mui/material/TableCell' @@ -52,6 +52,9 @@ import LiveView from '../LiveView/LiveView' import SessionData, { createSessionData } from '../../models/session-data' function descendingComparator (a: T, b: T, orderBy: keyof T): number { + if (orderBy === 'sessionDurationMillis') { + return Number(b[orderBy]) - Number(a[orderBy]) + } if (b[orderBy] < a[orderBy]) { return -1 } @@ -94,7 +97,7 @@ const headCells: HeadCell[] = [ { id: 'id', numeric: false, label: 'Session' }, { id: 'capabilities', numeric: false, label: 'Capabilities' }, { id: 'startTime', numeric: false, label: 'Start time' }, - { id: 'sessionDurationMillis', numeric: false, label: 'Duration' }, + { id: 'sessionDurationMillis', numeric: true, label: 'Duration' }, { id: 'nodeUri', numeric: false, label: 'Node URI' } ] @@ -170,13 +173,21 @@ function RunningSessions (props) { const [rowOpen, setRowOpen] = useState('') const [rowLiveViewOpen, setRowLiveViewOpen] = useState('') const [order, setOrder] = useState('asc') - const [orderBy, setOrderBy] = useState('startTime') + const [orderBy, setOrderBy] = useState('sessionDurationMillis') const [selected, setSelected] = useState([]) const [page, setPage] = useState(0) const [dense, setDense] = useState(false) const [rowsPerPage, setRowsPerPage] = useState(10) const [searchFilter, setSearchFilter] = useState('') const [searchBarHelpOpen, setSearchBarHelpOpen] = useState(false) + const liveViewRef = useRef(null) + + const handleDialogClose = () => { + if (liveViewRef.current) { + liveViewRef.current.disconnect() + } + setRowLiveViewOpen('') + } const handleRequestSort = (event: React.MouseEvent, property: keyof SessionData) => { @@ -379,6 +390,7 @@ function RunningSessions (props) { sx={{ height: '600px' }} > setRowLiveViewOpen('')} @@ -386,7 +398,7 @@ function RunningSessions (props) {