Merge branch 'trunk' into bidi_add_authentication_handler

aguspe · web-flow · commit 620ab24790d8 · 2024-10-16T17:06:41.000+02:00
diff --git a/java/spotbugs-excludes.xml b/java/spotbugs-excludes.xml
@@ -204,4 +204,30 @@
     <Class name="org.openqa.selenium.remote.internal.WebElementToJsonConverter" />
     <Bug pattern="NM_SAME_SIMPLE_NAME_AS_SUPERCLASS" />
   </Match>
+
+  <Match>
+    <Class name="org.openqa.selenium.chrome.AddHasCasting" />
+    <Bug pattern="NM_SAME_SIMPLE_NAME_AS_SUPERCLASS" />
+  </Match>
+
+  <Match>
+    <Class name="org.openqa.selenium.chrome.AddHasCdp" />
+    <Bug pattern="NM_SAME_SIMPLE_NAME_AS_SUPERCLASS" />
+  </Match>
+
+  <Match>
+    <Class name="org.openqa.selenium.edge.AddHasCdp" />
+    <Bug pattern="NM_SAME_SIMPLE_NAME_AS_SUPERCLASS" />
+  </Match>
+
+  <Match>
+    <Class name="org.openqa.selenium.edge.AddHasCasting" />
+    <Bug pattern="NM_SAME_SIMPLE_NAME_AS_SUPERCLASS" />
+  </Match>
+
+  <Match>
+    <Class name="~org.openqa.selenium.devtools.v[0-9]+\.v\w+" />
+    <Bug pattern="NM_CLASS_NAMING_CONVENTION"/>
+  </Match>
+
 </FindBugsFilter>
diff --git 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
@@ -127,7 +127,7 @@ public Either<HttpRequest, HttpResponse> 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
@@ -127,7 +127,7 @@ public Either<HttpRequest, HttpResponse> 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
@@ -127,7 +127,7 @@ public Either<HttpRequest, HttpResponse> 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
@@ -137,7 +137,7 @@ public Either<HttpRequest, HttpResponse> 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
@@ -214,6 +214,7 @@ public boolean equals(Object o) {
     return Objects.equals(this.nodeId, that.nodeId)
         && Objects.equals(this.externalUri, that.externalUri)
         && this.maxSessionCount == that.maxSessionCount
+        && this.sessionTimeout == that.sessionTimeout
         && Objects.equals(this.slots, that.slots)
         && Objects.equals(this.availability, that.availability)
         && Objects.equals(this.version, that.version);
@@ -224,7 +225,7 @@ public int hashCode() {
     return Objects.hash(nodeId, externalUri, maxSessionCount, slots, version);
   }
 
-  private Map<String, Object> toJson() {
+  public Map<String, Object> toJson() {
     Map<String, Object> 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
@@ -97,6 +97,7 @@ public List<Node> 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
@@ -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<Capabilities, Integer> stereotypes;
   private final Map<Session, Slot> activeSessions;
   private final String version;
@@ -49,6 +51,7 @@ public Node(
       URI uri,
       Availability status,
       int maxSession,
+      Duration sessionTimeout,
       int slotCount,
       Map<Capabilities, Integer> stereotypes,
       Map<Session, Slot> 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<org.openqa.selenium.grid.graphql.Session> 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<Session, Slot> 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
@@ -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/router/GridStatusHandler.java b/java/src/org/openqa/selenium/grid/router/GridStatusHandler.java
@@ -135,18 +135,7 @@ public HttpResponse execute(HttpRequest req) {
 
       List<Map<String, Object>> nodeResults =
           status.getNodes().stream()
-              .map(
-                  node ->
-                      new ImmutableMap.Builder<String, Object>()
-                          .put("id", node.getNodeId())
-                          .put("uri", node.getExternalUri())
-                          .put("maxSessions", node.getMaxSessionCount())
-                          .put("osInfo", node.getOsInfo())
-                          .put("heartbeatPeriod", node.getHeartbeatPeriod().toMillis())
-                          .put("availability", node.getAvailability())
-                          .put("version", node.getVersion())
-                          .put("slots", node.getSlots())
-                          .build())
+              .map(node -> new ImmutableMap.Builder<String, Object>().putAll(node.toJson()).build())
               .collect(toList());
 
       ImmutableMap.Builder<String, Object> value = ImmutableMap.builder();
diff --git 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
@@ -102,6 +102,7 @@ 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))
             .build();
 
     wait =
@@ -143,6 +144,7 @@ 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));
   }
 
   @Test
diff --git 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<HTMLDivElement, AlertProps>(function Alert (
   return <MuiAlert elevation={6} ref={ref} variant='filled' {...props} />
 })
 
-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) {
       </Snackbar>
     </div>
   )
-}
+})
 
 export default LiveView
diff --git 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<T> (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<Order>('asc')
-  const [orderBy, setOrderBy] = useState<keyof SessionData>('startTime')
+  const [orderBy, setOrderBy] = useState<keyof SessionData>('sessionDurationMillis')
   const [selected, setSelected] = useState<string[]>([])
   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<unknown>,
     property: keyof SessionData) => {
@@ -379,14 +390,15 @@ function RunningSessions (props) {
                                       sx={{ height: '600px' }}
                                     >
                                       <LiveView
+                                        ref={liveViewRef}
                                         url={row.vnc as string}
                                         scaleViewport
                                         onClose={() => setRowLiveViewOpen('')}
                                       />
                                     </DialogContent>
                                     <DialogActions>
                                       <Button
-                                        onClick={() => setRowLiveViewOpen('')}
+                                        onClick={handleDialogClose}
                                         color='primary'
                                         variant='contained'
                                       >
diff --git a/py/selenium/webdriver/remote/remote_connection.py b/py/selenium/webdriver/remote/remote_connection.py
@@ -336,7 +336,9 @@ def _request(self, method, url, body=None):
             if 300 <= statuscode < 304:
                 return self._request("GET", response.headers.get("location", None))
             if 399 < statuscode <= 500:
-                return {"status": statuscode, "value": data}
+                if statuscode == 401:
+                    return {"status": statuscode, "value": "Authorization Required"}
+                return {"status": statuscode, "value": str(statuscode) if not data else data.strip()}
             content_type = []
             if response.headers.get("Content-Type", None):
                 content_type = response.headers.get("Content-Type", None).split(";")

Original file line number	Diff line number	Diff line change
`@@ -127,7 +127,7 @@ public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedRe`
`127`	`127`	`} catch (DevToolsException e) {`
`128`	`128`	`// Redirects don't seem to have bodies`
`129`	`129`	`int code = pausedReq.getResponseStatusCode().orElse(HTTP_OK);`
`130`		`- if (code < 300 && code > 399) {`
	`130`	`+ if (code >= 300 && code <= 399) {`
`131`	`131`	`LOG.warning("Unable to get body for request id " + pausedReq.getRequestId());`
`132`	`132`	`}`
`133`	`133`
Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedRe`
`137`	`137`	`} catch (DevToolsException e) {`
`138`	`138`	`// Redirects don't seem to have bodies`
`139`	`139`	`int code = pausedReq.getResponseStatusCode().orElse(HTTP_OK);`
`140`		`- if (code < 300 && code > 399) {`
	`140`	`+ if (code >= 300 && code <= 399) {`
`141`	`141`	`LOG.warning("Unable to get body for request id " + pausedReq.getRequestId());`
`142`	`142`	`}`
`143`	`143`