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 <trev.norris@gmail.com>
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>
@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 <trev.norris@gmail.com>
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>

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.

None yet

5 participants