Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

net: throw on invalid socket timeouts #8884

Closed
wants to merge 1 commit into from
Closed

net: throw on invalid socket timeouts #8884

wants to merge 1 commit into from

Conversation

cjihrig
Copy link

@cjihrig cjihrig commented Dec 16, 2014

This commit restricts socket timeouts non-negative, finite numbers. Any other value throws a TypeError or RangeError. This prevents subtle bugs that can happen due to type coercion.

@misterdjules @tjfontaine

This commit restricts socket timeouts non-negative, finite
numbers. Any other value throws a TypeError or RangeError.
This prevents subtle bugs that can happen due to type
coercion.
@trevnorris
Copy link

Haven't tested it, but the code looks good. LGTM.

@tjfontaine
Copy link

LGTM as well

cjihrig added a commit that referenced this pull request Jan 22, 2015
This commit restricts socket timeouts non-negative, finite
numbers. Any other value throws a TypeError or RangeError.
This prevents subtle bugs that can happen due to type
coercion.

Fixes: #8618
PR-URL: #8884
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Timothy J Fontaine <[email protected]>
@cjihrig
Copy link
Author

cjihrig commented Jan 22, 2015

Landed in f347573

@cjihrig cjihrig closed this Jan 22, 2015
@cjihrig cjihrig deleted the timers branch January 22, 2015 18:28
cjihrig added a commit to nodejs/node that referenced this pull request Feb 13, 2015
This commit restricts socket timeouts non-negative, finite
numbers. Any other value throws a TypeError or RangeError.
This prevents subtle bugs that can happen due to type
coercion.

Fixes: nodejs/node-v0.x-archive#8618
PR-URL: nodejs/node-v0.x-archive#8884
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Timothy J Fontaine <[email protected]>

Conflicts:
	lib/timers.js
	test/simple/test-net-settimeout.js
	test/simple/test-net-socket-timeout.js
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants