Bug 181473 – WebDriver: deserializeTimeouts() shouldn't reject double timeout values

• Component: WebDriver
• Status: RESOLVED FIXED

[Zan Dobersek]

WebDriver: deserializeTimeouts() shouldn't reject double timeout values

[Zan Dobersek]

Created attachment 330891 [details]
Patch

[Carlos Garcia Campos]

8.5 Set Timeouts

The steps to deserialize as a timeout with argument parameters are:

2.4. If value is not an integer, or it is less than 0 or greater than 264 – 1, return error with error code invalid argument.

[Carlos Garcia Campos]

Comment on attachment 330891 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330891&action=review

We should check that in case of being a double, it's an integer as the spec says. It's probably worht it adding a helper function isIntegerValue() since there are more integers mentioned in the spec

> Source/WebDriver/WebDriverService.cpp:284
> -        if (it->value->type() != JSON::Value::Type::Integer || !it->value->asInteger(timeoutMS) || timeoutMS < 0 || timeoutMS > INT_MAX)
> +        if (!it->value->asInteger(timeoutMS) || timeoutMS < 0 || timeoutMS > INT_MAX)

I didn't know the spec also says:

"An integer is a Number that is unchanged under the ToInteger operation."

But this would allow any double which would break imported/w3c/webdriver/tests/sessions/new_session/invalid_capabilities.py::test_invalid_values

[Zan Dobersek]

Created attachment 330902 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 330902 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330902&action=review

Excellent, thanks!

> Source/WebDriver/ChangeLog:7
> +

Please, mention here the tests fixed.

> Source/WebDriver/WebDriverService.cpp:280
> +    // value, i.e. if the double value was not originally in integer form.

Add also a link to the spec https://w3c.github.io/webdriver/webdriver-spec.html#dfn-integer

[Zan Dobersek]

Created attachment 330907 [details]
Patch for landing

[Zan Dobersek]

Comment on attachment 330907 [details]
Patch for landing

Clearing flags on attachment: 330907

Committed r226716: <https://trac.webkit.org/changeset/226716>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/36406613>