cast to (int) for Curl adapter timeout config #121
Conversation
|
What is the reason for accepting non-integer timeout values? |
|
just for consistent with socket adapter. if it should be get exception, the in socket adapter should get exception as well? |
|
That would be my preference, yes. I will need to look into it sometime later, may be i don't see something. |
src/Client/Adapter/Curl.php
Outdated
| @@ -203,9 +203,9 @@ public function connect($host, $port = 80, $secure = false) | |||
| } | |||
|
|
|||
| if (isset($this->config['connecttimeout'])) { | |||
| $connectTimeout = $this->config['connecttimeout']; | |||
| $connectTimeout = (int) $this->config['connecttimeout']; | |||
There was a problem hiding this comment.
I would prefer the following approach:
- Grab the config value.
- If it is not an integer or not a numeric string, throw an
InvalidArgumentException. - Cast to integer.
That approach ensures strings like timeout do not get cast to 0, which is obviously incorrect.
There was a problem hiding this comment.
do we need to check $this->config['timeout'] and $this->config['connecttimeout'] in setOptions() function ? and change it at Socket adapter as well?
There was a problem hiding this comment.
I've updated with integer and numeric check, applied to socket adapter as well.
samsonasik
force-pushed
the
curl-timeout-int
branch
from
353ec89
to
5b36ca3
Compare
cast to (int) for Curl adapter timeout config
|
Thanks, @samsonasik! |
same as what applied into Socket adapter