Merge branch 'trunk' into update-renovate-json

Author: DineshKumarRA

dotnet/src/webdriver/Firefox/FirefoxProfile.cs

@@ -296,24 +296,14 @@ private void UpdateUserPreferences()
 
         private void ReadDefaultPreferences()
         {
-            var jsonSerializerOptions = new JsonSerializerOptions
-            {
-                Converters =
-                {
-                    new ResponseValueJsonConverter()
-                }
-            };
-
             using (Stream defaultPrefsStream = ResourceUtilities.GetResourceStream("webdriver_prefs.json", "webdriver_prefs.json"))
             {
-                using (StreamReader reader = new StreamReader(defaultPrefsStream))
-                {
-                    string defaultPreferences = reader.ReadToEnd();
-                    Dictionary<string, object> deserializedPreferences = JsonSerializer.Deserialize<Dictionary<string, object>>(defaultPreferences, jsonSerializerOptions);
-                    Dictionary<string, object> immutableDefaultPreferences = deserializedPreferences["frozen"] as Dictionary<string, object>;
-                    Dictionary<string, object> editableDefaultPreferences = deserializedPreferences["mutable"] as Dictionary<string, object>;
-                    this.profilePreferences = new Preferences(immutableDefaultPreferences, editableDefaultPreferences);
-                }
+                using JsonDocument defaultPreferences = JsonDocument.Parse(defaultPrefsStream);
+
+                JsonElement immutableDefaultPreferences = defaultPreferences.RootElement.GetProperty("frozen");
+                JsonElement editableDefaultPreferences = defaultPreferences.RootElement.GetProperty("mutable");
+
+                this.profilePreferences = new Preferences(immutableDefaultPreferences, editableDefaultPreferences);
             }
         }
 

dotnet/src/webdriver/Firefox/Preferences.cs

@@ -21,6 +21,9 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
+using System.Text.Json;
+
+#nullable enable
 
 namespace OpenQA.Selenium.Firefox
 {
@@ -29,31 +32,27 @@ namespace OpenQA.Selenium.Firefox
     /// </summary>
     internal class Preferences
     {
-        private Dictionary<string, string> preferences = new Dictionary<string, string>();
-        private Dictionary<string, string> immutablePreferences = new Dictionary<string, string>();
+        private readonly Dictionary<string, string> preferences = new Dictionary<string, string>();
+        private readonly HashSet<string> immutablePreferences = new HashSet<string>();
 
         /// <summary>
         /// Initializes a new instance of the <see cref="Preferences"/> class.
         /// </summary>
         /// <param name="defaultImmutablePreferences">A set of preferences that cannot be modified once set.</param>
         /// <param name="defaultPreferences">A set of default preferences.</param>
-        public Preferences(Dictionary<string, object> defaultImmutablePreferences, Dictionary<string, object> defaultPreferences)
+        public Preferences(JsonElement defaultImmutablePreferences, JsonElement defaultPreferences)
         {
-            if (defaultImmutablePreferences != null)
+            foreach (JsonProperty pref in defaultImmutablePreferences.EnumerateObject())
             {
-                foreach (KeyValuePair<string, object> pref in defaultImmutablePreferences)
-                {
-                    this.SetPreferenceValue(pref.Key, pref.Value);
-                    this.immutablePreferences.Add(pref.Key, pref.Value.ToString());
-                }
+                this.ThrowIfPreferenceIsImmutable(pref.Name, pref.Value);
+                this.preferences[pref.Name] = pref.Value.GetRawText();
+                this.immutablePreferences.Add(pref.Name);
             }
 
-            if (defaultPreferences != null)
+            foreach (JsonProperty pref in defaultPreferences.EnumerateObject())
             {
-                foreach (KeyValuePair<string, object> pref in defaultPreferences)
-                {
-                    this.SetPreferenceValue(pref.Key, pref.Value);
-                }
+                this.ThrowIfPreferenceIsImmutable(pref.Name, pref.Value);
+                this.preferences[pref.Name] = pref.Value.GetRawText();
             }
         }
 
@@ -64,9 +63,31 @@ public Preferences(Dictionary<string, object> defaultImmutablePreferences, Dicti
         /// <param name="value">A <see cref="string"/> value give the preference.</param>
         /// <remarks>If the preference already exists in the currently-set list of preferences,
         /// the value will be updated.</remarks>
+        /// <exception cref="ArgumentNullException">If <paramref name="key"/> or <paramref name="value"/> are <see langword="null"/>.</exception>
+        /// <exception cref="ArgumentException">
+        /// <para>If <paramref name="value"/> is wrapped with double-quotes.</para>
+        /// <para>-or-</para>
+        /// <para>If the specified preference is immutable.</para>
+        /// </exception>
         internal void SetPreference(string key, string value)
         {
-            this.SetPreferenceValue(key, value);
+            if (key is null)
+            {
+                throw new ArgumentNullException(nameof(key));
+            }
+
+            if (value is null)
+            {
+                throw new ArgumentNullException(nameof(value));
+            }
+
+            if (IsWrappedAsString(value))
+            {
+                throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Preference values must be plain strings: {0}: {1}", key, value));
+            }
+
+            this.ThrowIfPreferenceIsImmutable(key, value);
+            this.preferences[key] = string.Format(CultureInfo.InvariantCulture, "\"{0}\"", value);
         }
 
         /// <summary>
@@ -76,9 +97,17 @@ internal void SetPreference(string key, string value)
         /// <param name="value">A <see cref="int"/> value give the preference.</param>
         /// <remarks>If the preference already exists in the currently-set list of preferences,
         /// the value will be updated.</remarks>
+        /// <exception cref="ArgumentNullException">If <paramref name="key"/> is <see langword="null"/>.</exception>
+        /// <exception cref="ArgumentException">If the specified preference is immutable.</exception>
         internal void SetPreference(string key, int value)
         {
-            this.SetPreferenceValue(key, value);
+            if (key is null)
+            {
+                throw new ArgumentNullException(nameof(key));
+            }
+
+            this.ThrowIfPreferenceIsImmutable(key, value);
+            this.preferences[key] = value.ToString(CultureInfo.InvariantCulture);
         }
 
         /// <summary>
@@ -88,16 +117,25 @@ internal void SetPreference(string key, int value)
         /// <param name="value">A <see cref="bool"/> value give the preference.</param>
         /// <remarks>If the preference already exists in the currently-set list of preferences,
         /// the value will be updated.</remarks>
+        /// <exception cref="ArgumentNullException">If <paramref name="key"/> is <see langword="null"/>.</exception>
+        /// <exception cref="ArgumentException">If the specified preference is immutable.</exception>
         internal void SetPreference(string key, bool value)
         {
-            this.SetPreferenceValue(key, value);
+            if (key is null)
+            {
+                throw new ArgumentNullException(nameof(key));
+            }
+
+            this.ThrowIfPreferenceIsImmutable(key, value);
+            this.preferences[key] = value ? "true" : "false";
         }
 
         /// <summary>
         /// Gets a preference from the list of preferences.
         /// </summary>
         /// <param name="preferenceName">The name of the preference to retrieve.</param>
         /// <returns>The value of the preference, or an empty string if the preference is not set.</returns>
+        /// <exception cref="ArgumentNullException">If <paramref name="preferenceName"/> is <see langword="null"/>.</exception>
         internal string GetPreference(string preferenceName)
         {
             if (this.preferences.ContainsKey(preferenceName))
@@ -151,44 +189,18 @@ private static bool IsWrappedAsString(string value)
             return value.StartsWith("\"", StringComparison.OrdinalIgnoreCase) && value.EndsWith("\"", StringComparison.OrdinalIgnoreCase);
         }
 
-        private bool IsSettablePreference(string preferenceName)
+        private void ThrowIfPreferenceIsImmutable<TValue>(string preferenceName, TValue value)
         {
-            return !this.immutablePreferences.ContainsKey(preferenceName);
-        }
-
-        private void SetPreferenceValue(string key, object value)
-        {
-            if (!this.IsSettablePreference(key))
+            if (this.immutablePreferences.Contains(preferenceName))
             {
-                string message = string.Format(CultureInfo.InvariantCulture, "Preference {0} may not be overridden: frozen value={1}, requested value={2}", key, this.immutablePreferences[key], value.ToString());
+                string message = string.Format(CultureInfo.InvariantCulture, "Preference {0} may not be overridden: frozen value={1}, requested value={2}", preferenceName, this.preferences[preferenceName], value?.ToString());
                 throw new ArgumentException(message);
             }
+        }
 
-            string stringValue = value as string;
-            if (stringValue != null)
-            {
-                if (IsWrappedAsString(stringValue))
-                {
-                    throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Preference values must be plain strings: {0}: {1}", key, value));
-                }
-
-                this.preferences[key] = string.Format(CultureInfo.InvariantCulture, "\"{0}\"", value);
-                return;
-            }
-
-            if (value is bool)
-            {
-                this.preferences[key] = Convert.ToBoolean(value, CultureInfo.InvariantCulture).ToString().ToLowerInvariant();
-                return;
-            }
-
-            if (value is int || value is long)
-            {
-                this.preferences[key] = Convert.ToInt32(value, CultureInfo.InvariantCulture).ToString(CultureInfo.InvariantCulture);
-                return;
-            }
-
-            throw new WebDriverException("Value must be string, int or boolean");
+        private bool IsSettablePreference(string preferenceName)
+        {
+            return !this.immutablePreferences.Contains(preferenceName);
         }
     }
 }

java/spotbugs-excludes.xml

@@ -230,4 +230,9 @@
     <Bug pattern="NM_CLASS_NAMING_CONVENTION"/>
   </Match>
 
+  <Match>
+    <Class name="org.openqa.selenium.manager.SeleniumManager"/>
+    <Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
+  </Match>
+
 </FindBugsFilter>

java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java

@@ -27,6 +27,7 @@
 import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
 import static org.openqa.selenium.remote.RemoteTags.SESSION_ID;
 import static org.openqa.selenium.remote.RemoteTags.SESSION_ID_EVENT;
+import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
 import static org.openqa.selenium.remote.tracing.AttributeKey.SESSION_URI;
 import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
 
@@ -79,6 +80,7 @@
 import org.openqa.selenium.grid.data.NodeStatus;
 import org.openqa.selenium.grid.data.NodeStatusEvent;
 import org.openqa.selenium.grid.data.RequestId;
+import org.openqa.selenium.grid.data.Session;
 import org.openqa.selenium.grid.data.SessionRequest;
 import org.openqa.selenium.grid.data.SessionRequestCapability;
 import org.openqa.selenium.grid.data.Slot;
@@ -109,6 +111,7 @@
 import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.remote.SessionId;
 import org.openqa.selenium.remote.http.HttpClient;
+import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.tracing.AttributeKey;
 import org.openqa.selenium.remote.tracing.AttributeMap;
 import org.openqa.selenium.remote.tracing.Span;
@@ -853,21 +856,34 @@ private void handleNewSessionRequest(SessionRequest sessionRequest) {
           }
         }
 
-        // 'complete' will return 'true' if the session has not timed out during the creation
-        // process: it's still a valid session as it can be used by the client
         boolean isSessionValid = sessionQueue.complete(reqId, response);
-        // If the session request has timed out, tell the Node to remove the session, so that does
-        // not stall
+        // terminate invalid sessions to avoid stale sessions
         if (!isSessionValid && response.isRight()) {
           LOG.log(
               Level.INFO,
-              "Session for request {0} has been created but it has timed out, stopping it to avoid"
-                  + " stalled browser",
+              "Session for request {0} has been created but it has timed out or the connection"
+                  + " dropped, stopping it to avoid stalled browser",
               reqId.toString());
-          URI nodeURI = response.right().getSession().getUri();
-          Node node = getNodeFromURI(nodeURI);
+          Session session = response.right().getSession();
+          Node node = getNodeFromURI(session.getUri());
           if (node != null) {
-            node.stop(response.right().getSession().getId());
+            boolean deleted;
+            try {
+              // Attempt to stop the session
+              deleted =
+                  node.execute(new HttpRequest(DELETE, "/session/" + session.getId())).getStatus()
+                      == 200;
+            } catch (Exception e) {
+              LOG.log(
+                  Level.WARNING,
+                  String.format("Exception while trying to delete session %s", session.getId()),
+                  e);
+              deleted = false;
+            }
+            if (!deleted) {
+              // Kill the session
+              node.stop(session.getId());
+            }
           }
         }
       }

java/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java

@@ -220,10 +220,14 @@ public HttpResponse addToQueue(SessionRequest request) {
           result = Either.left(new SessionNotCreatedException("New session request timed out"));
         }
       } catch (InterruptedException e) {
+        // the client will never see the session, ensure the session is disposed
+        data.cancel();
         Thread.currentThread().interrupt();
         result =
             Either.left(new SessionNotCreatedException("Interrupted when creating the session", e));
       } catch (RuntimeException e) {
+        // the client will never see the session, ensure the session is disposed
+        data.cancel();
         result =
             Either.left(
                 new SessionNotCreatedException("An error occurred creating the session", e));
@@ -367,43 +371,30 @@ public List<SessionRequest> getNextAvailable(Map<Capabilities, Long> stereotypes
     }
   }
 
-  /** Returns true if the session is still valid (not timed out) */
+  /** Returns true if the session is still valid (not timed out and not canceled) */
   @Override
   public boolean complete(
       RequestId reqId, Either<SessionNotCreatedException, CreateSessionResponse> result) {
     Require.nonNull("New session request", reqId);
     Require.nonNull("Result", result);
     TraceContext context = contexts.getOrDefault(reqId, tracer.getCurrentContext());
     try (Span ignored = context.createSpan("sessionqueue.completed")) {
-      Lock readLock = lock.readLock();
-      readLock.lock();
       Data data;
-      boolean isSessionTimedOut = false;
-      try {
-        data = requests.get(reqId);
-      } finally {
-        readLock.unlock();
-      }
-
-      if (data == null) {
-        return false;
-      } else {
-        isSessionTimedOut = isTimedOut(Instant.now(), data);
-      }
-
       Lock writeLock = lock.writeLock();
       writeLock.lock();
       try {
-        requests.remove(reqId);
+        data = requests.remove(reqId);
         queue.removeIf(req -> reqId.equals(req.getRequestId()));
         contexts.remove(reqId);
       } finally {
         writeLock.unlock();
       }
 
-      data.setResult(result);
+      if (data == null) {
+        return false;
+      }
 
-      return !isSessionTimedOut;
+      return data.setResult(result);
     }
   }
 
@@ -466,6 +457,7 @@ private class Data {
     private final CountDownLatch latch = new CountDownLatch(1);
     private Either<SessionNotCreatedException, CreateSessionResponse> result;
     private boolean complete;
+    private boolean canceled;
 
     public Data(Instant enqueued) {
       this.endTime = enqueued.plus(requestTimeout);
@@ -476,14 +468,19 @@ public synchronized Either<SessionNotCreatedException, CreateSessionResponse> ge
       return result;
     }
 
-    public synchronized void setResult(
+    public synchronized void cancel() {
+      canceled = true;
+    }
+
+    public synchronized boolean setResult(
         Either<SessionNotCreatedException, CreateSessionResponse> result) {
-      if (complete) {
-        return;
+      if (complete || canceled) {
+        return false;
       }
       this.result = result;
       complete = true;
       latch.countDown();
+      return true;
     }
   }
 }

java/src/org/openqa/selenium/manager/SeleniumManager.java

@@ -213,7 +213,7 @@ private synchronized Path getBinary() {
         if (!binary.toFile().exists()) {
           String binaryPathInJar = String.format("%s/%s%s", folder, SELENIUM_MANAGER, extension);
           try (InputStream inputStream = this.getClass().getResourceAsStream(binaryPathInJar)) {
-            binary.getParent().toFile().mkdirs();
+            Files.createDirectories(binary.getParent());
             Files.copy(inputStream, binary);
           }
         }