Skip to content

Commit

Permalink
Allowing null value for script timeout in IE
Browse files Browse the repository at this point in the history
The W3C WebDriver Specification has contradictory language regarding
the script timeout. On the one hand, it specifies that the value of
the script timeout can be `null`, indicating an indefinite timeout.
On the other hand, it specifies that when setting or getting timeouts,
the value of any timeout must be an integer between 0 and 2^53 - 1.
Since geckodriver has made the assumption that the former condition
is the correct interpretation, and the maintainers of geckodriver have
added a test to the W3C Web Platform Tests for WebDriver that expects
this interpretation, the IE driver is being modified to follow that
decision. It is the opinion of the developers of the IE driver that
this is the incorrect interpretation, but there is no recourse to have
the geckodriver maintainers revisit their decision.
  • Loading branch information
jimevans committed Jan 24, 2019
1 parent 269c234 commit 7c87e88
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 29 deletions.
6 changes: 5 additions & 1 deletion cpp/iedriver/CommandHandlers/GetTimeoutsCommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ void GetTimeoutsCommandHandler::ExecuteInternal(
Response* response) {
Json::Value response_value;
response_value[IMPLICIT_WAIT_TIMEOUT_NAME] = executor.implicit_wait_timeout();
response_value[SCRIPT_TIMEOUT_NAME] = executor.async_script_timeout();
if (executor.async_script_timeout() < 0) {
response_value[SCRIPT_TIMEOUT_NAME] = Json::Value::null;
} else {
response_value[SCRIPT_TIMEOUT_NAME] = executor.async_script_timeout();
}
response_value[PAGE_LOAD_TIMEOUT_NAME] = executor.page_load_timeout();
response->SetSuccessResponse(response_value);
}
Expand Down
58 changes: 35 additions & 23 deletions cpp/iedriver/CommandHandlers/NewSessionCommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,11 @@ void NewSessionCommandHandler::SetTimeoutSettings(const IECommandExecutor& execu
mutable_executor.set_page_load_timeout(capabilities[PAGE_LOAD_TIMEOUT_NAME].asUInt64());
}
if (capabilities.isMember(SCRIPT_TIMEOUT_NAME)) {
mutable_executor.set_async_script_timeout(capabilities[SCRIPT_TIMEOUT_NAME].asUInt64());
if (capabilities[SCRIPT_TIMEOUT_NAME].isNull()) {
mutable_executor.set_async_script_timeout(-1);
} else {
mutable_executor.set_async_script_timeout(capabilities[SCRIPT_TIMEOUT_NAME].asInt64());
}
}
}

Expand Down Expand Up @@ -490,7 +494,12 @@ Json::Value NewSessionCommandHandler::CreateReturnedCapabilities(const IECommand
Json::Value timeouts;
timeouts[IMPLICIT_WAIT_TIMEOUT_NAME] = executor.implicit_wait_timeout();
timeouts[PAGE_LOAD_TIMEOUT_NAME] = executor.page_load_timeout();
timeouts[SCRIPT_TIMEOUT_NAME] = executor.async_script_timeout();
long long script_timeout = executor.async_script_timeout();
if (script_timeout < 0) {
timeouts[SCRIPT_TIMEOUT_NAME] = Json::Value::null;
} else {
timeouts[SCRIPT_TIMEOUT_NAME] = script_timeout;
}
capabilities[TIMEOUTS_CAPABILITY] = timeouts;

Json::Value ie_options;
Expand Down Expand Up @@ -783,27 +792,30 @@ bool NewSessionCommandHandler::ValidateCapabilities(
}
std::string timeout_error = "";
Json::Value timeout_value = timeouts[timeout_name];
if (!timeout_value.isNumeric() || !timeout_value.isIntegral()) {
*error_message = "Invalid capabilities in " +
capability_set_name + ": " +
"timeout " + timeout_name +
"must be an integer";
return false;
}
if (!timeout_value.isInt64()) {
*error_message = "Invalid capabilities in " +
capability_set_name + ": " +
"timeout " + timeout_name +
"must be an integer between 0 and 2^53 - 1";
return false;
}
long long timeout = timeout_value.asInt64();
if (timeout < 0 || timeout > MAX_SAFE_INTEGER) {
*error_message = "Invalid capabilities in " +
capability_set_name + ": " +
"timeout " + timeout_name +
"must be an integer between 0 and 2^53 - 1";
return false;
// Special case: script timeout may be null.
if (timeout_name != SCRIPT_TIMEOUT_NAME || !timeout_value.isNull()) {
if (!timeout_value.isNumeric() || !timeout_value.isIntegral()) {
*error_message = "Invalid capabilities in " +
capability_set_name + ": " +
"timeout " + timeout_name +
"must be an integer";
return false;
}
if (!timeout_value.isInt64()) {
*error_message = "Invalid capabilities in " +
capability_set_name + ": " +
"timeout " + timeout_name +
"must be an integer between 0 and 2^53 - 1";
return false;
}
long long timeout = timeout_value.asInt64();
if (timeout < 0 || timeout > MAX_SAFE_INTEGER) {
*error_message = "Invalid capabilities in " +
capability_set_name + ": " +
"timeout " + timeout_name +
"must be an integer between 0 and 2^53 - 1";
return false;
}
}
}
}
Expand Down
14 changes: 11 additions & 3 deletions cpp/iedriver/CommandHandlers/SetTimeoutsCommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,25 @@ void SetTimeoutsCommandHandler::ExecuteInternal(
if (timeout_type != IMPLICIT_WAIT_TIMEOUT_NAME &&
timeout_type != SCRIPT_TIMEOUT_NAME &&
timeout_type != PAGE_LOAD_TIMEOUT_NAME) {
response->SetErrorResponse(ERROR_INVALID_ARGUMENT, "Invalid timeout type specified: " + timeout_type);
response->SetErrorResponse(ERROR_INVALID_ARGUMENT,
"Invalid timeout type specified: " + timeout_type);
return;
}
if (timeout_type == SCRIPT_TIMEOUT_NAME && timeout_value.isNull()) {
// Special case for the script timeout, which is nullable.
mutable_executor.set_async_script_timeout(-1);
return;
}
if (!timeout_value.isNumeric() ||
!timeout_value.isIntegral()) {
response->SetErrorResponse(ERROR_INVALID_ARGUMENT, "Timeout value for timeout type " + timeout_type + " must be an integer");
response->SetErrorResponse(ERROR_INVALID_ARGUMENT,
"Timeout value for timeout type " + timeout_type + " must be an integer");
return;
}
timeout = timeout_value.asInt64();
if (timeout < 0 || timeout > MAX_SAFE_INTEGER) {
response->SetErrorResponse(ERROR_INVALID_ARGUMENT, "Timeout value for timeout type " + timeout_type + " must be an integer between 0 and 2^53 - 1");
response->SetErrorResponse(ERROR_INVALID_ARGUMENT,
"Timeout value for timeout type " + timeout_type + " must be an integer between 0 and 2^53 - 1");
return;
}
if (timeout_type == IMPLICIT_WAIT_TIMEOUT_NAME) {
Expand Down
4 changes: 2 additions & 2 deletions cpp/iedriver/IECommandExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ class IECommandExecutor : public CWindowImpl<IECommandExecutor>, public IElement
this->implicit_wait_timeout_ = timeout;
}

unsigned long long async_script_timeout(void) const { return this->async_script_timeout_; }
void set_async_script_timeout(const unsigned long long timeout) {
long long async_script_timeout(void) const { return this->async_script_timeout_; }
void set_async_script_timeout(const long long timeout) {
this->async_script_timeout_ = timeout;
}

Expand Down

0 comments on commit 7c87e88

Please sign in to comment.