Do not leak DNS Request IDs #4981
Conversation
While jruby@d1a760e fixed handling of compressed IPv6 addresses, it also broke the "freeing" part of it. Currently every DNS request leaks single request id: ``` require 'resolv' Resolv::DNS::RequestID.values.map(&:length) Resolv::DNS.new.getaddress('example.com') Resolv::DNS::RequestID.values.map(&:length) ``` Given the fact that request ids are chosen from `0x0000..0xffff` range - the app can issue 65535 requests and then will be blocked forever trying to allocate another one. This commit makes request id caching work by using same data for allocation and freeing.
|
We can cover the problematic behavior in spect, but it requires either having a working DNS server to run the tests (ie: move the example above into a spec) or adding a spec for internal classes/methods (not sure if it would be useful, though) |
|
@fxposter have you tried this against MRI? This code is theirs. If it affects them then we should get this moved upstream. |
|
This is not strictly true. The commit mentioned above diverges from the MRI behavior which don't have any Hence they don't leak request ids here. |
|
@fxposter oh sorry I should have checked. See: jruby/ruby@44fe41b#diff-5eaee83b511bb5c304d686f8f47dc6c2L773 That commit shows we maintain doing that in both sender and recv_reply. Note this is 2.3.x support but I should have realized you probably wanted this PR against jruby-9.1 branch and not master since master is for 2.4.x support atm (well no doubt you will want it in all the places but we have not put out jruby 9.2 yet). Could I ask two favors?
The great thing about this work is fixing up part of our MRI stdlib changeset. One less difference is one less headache at some point in the future :) |
|
this has been stale for a while but seems that this makes sense to go into 9.2 ... marking for now |
While d1a760e fixed handling of compressed IPv6 addresses,
it also broke the "freeing" part of it.
Currently every DNS request leaks single request id:
Given the fact that request ids are chosen from
0x0000..0xffffrange - the app can issue 65535 requests and then will be blocked forever trying to allocate another one.This commit makes request id caching work by using same data for allocation and freeing.