From ad55419b406d580e811f4d0c7240107f83b26bf6 Mon Sep 17 00:00:00 2001
From: Marc Fisher <fisherii@google.com>
Date: Thu, 23 Mar 2017 14:08:31 -0700
Subject: [PATCH] Throw an exception if new session returns an error.

Previously if the http response code was not an error
response, then an exception would not be thrown even
if the response contained an error field or a non-zero
status. This was specifically a problem for Chromedriver.
---
 .../selenium/remote/ProtocolHandshake.java    | 52 ++++++++++---------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/java/client/src/org/openqa/selenium/remote/ProtocolHandshake.java b/java/client/src/org/openqa/selenium/remote/ProtocolHandshake.java
index 019e8a98008d4..b5ec4172d99b8 100644
--- a/java/client/src/org/openqa/selenium/remote/ProtocolHandshake.java
+++ b/java/client/src/org/openqa/selenium/remote/ProtocolHandshake.java
@@ -314,11 +314,32 @@ private Optional<Result> createSession(HttpClient client, InputStream newSession
       jsonBlob = new HashMap<>();
     }
 
-    // If the result looks positive, return the result.
-    Object sessionId = jsonBlob.get("sessionId");
     Object value = jsonBlob.get("value");
     Object w3cError = jsonBlob.get("error");
     Object ossStatus = jsonBlob.get("status");
+
+    // If the result was an error that we believe has to do with the remote end failing to start the
+    // session, create an exception and throw it.
+    Response tempResponse = null;
+    if (response.getStatus() != HttpURLConnection.HTTP_OK) {
+      tempResponse = new Response(null);
+      tempResponse.setStatus(ErrorCodes.SESSION_NOT_CREATED);
+      tempResponse.setValue(value);
+    } else if ("session not created".equals(w3cError)) {
+      tempResponse = new Response(null);
+      tempResponse.setStatus(ErrorCodes.SESSION_NOT_CREATED);
+      tempResponse.setValue(value);
+    } else if (ossStatus instanceof Number && ((Number) ossStatus).intValue() != 0) {
+      tempResponse = new Response(null);
+      tempResponse.setStatus(ErrorCodes.SESSION_NOT_CREATED);
+      tempResponse.setValue(value);
+    }
+
+    if (tempResponse != null) {
+      new ErrorHandler().throwIfResponseFailed(tempResponse, 0);
+    }
+
+    Object sessionId = jsonBlob.get("sessionId");
     Map<String, ?> capabilities = null;
 
     // The old geckodriver prior to 0.14 returned "value" as the thing containing the session id.
@@ -342,29 +363,10 @@ private Optional<Result> createSession(HttpClient client, InputStream newSession
       capabilities = ((Capabilities) capabilities).asMap();
     }
 
-    if (response.getStatus() == HttpURLConnection.HTTP_OK) {
-      if (sessionId != null && capabilities != null) {
-        Dialect dialect = ossStatus == null ? Dialect.W3C : Dialect.OSS;
-        return Optional.of(
-          new Result(dialect, String.valueOf(sessionId), capabilities));
-      }
-    }
-
-    // If the result was an error that we believe has to do with the remote end failing to start the
-    // session, create an exception and throw it.
-    Response tempResponse = null;
-    if ("session not created".equals(w3cError)) {
-      tempResponse = new Response(null);
-      tempResponse.setStatus(ErrorCodes.SESSION_NOT_CREATED);
-      tempResponse.setValue(jsonBlob);
-    } else if (ossStatus instanceof Number) {
-      tempResponse = new Response(null);
-      tempResponse.setStatus(ErrorCodes.SESSION_NOT_CREATED);
-      tempResponse.setValue(jsonBlob);
-    }
-
-    if (tempResponse != null) {
-      new ErrorHandler().throwIfResponseFailed(tempResponse, 0);
+    // If the result looks positive, return the result.
+    if (sessionId != null && capabilities != null) {
+      Dialect dialect = ossStatus == null ? Dialect.W3C : Dialect.OSS;
+      return Optional.of(new Result(dialect, String.valueOf(sessionId), capabilities));
     }
 
     // Otherwise, just return empty.