Add support for Client Subnet in DNS Queries (RFC7871)#5614
Add support for Client Subnet in DNS Queries (RFC7871)#5614normanmaurer wants to merge 1 commit into4.1from
Conversation
|
@Scottmitch PTAL |
| // See https://tools.ietf.org/html/rfc6891#section-6.1.3 | ||
| private static long packIntoLong(int val, int val2) { | ||
| // We are currently not support DO and Z fields, just use 0. | ||
| return ((val & 0xff) << 24 | (val2 & 0xff) << 16 | (0 & 0xff) << 8 | 0 & 0xff) & 0xFFFFFFFFL; |
There was a problem hiding this comment.
can we just do ((val & 0xff) << 24 | (val2 & 0xff) << 16) for now?
There was a problem hiding this comment.
I would prefer to keep it as it is to make it "easier" to add support later. Its not a big deal imho to keep one more shift.
There was a problem hiding this comment.
not a big deal ... also not hard to add later ... fine as as
| // See http://www.iana.org/assignments/address-family-numbers/address-family-numbers.xhtml | ||
| final short addressNumber; | ||
| if (address instanceof Inet4Address) { | ||
| addressNumber = 1; |
There was a problem hiding this comment.
can we extend InternetProtocolFamily to include this information?
There was a problem hiding this comment.
reminder: use the new addition to InternetProtocolFamily here
normanmaurer
force-pushed
the
dns_rcs
branch
2 times, most recently
from
110b8cd to
3fd962a
Compare
| out.writeByte(scopePrefixLength); // Must be 0 in queries. | ||
|
|
||
| if (leftOverBits > 0) { | ||
| int bytesLength = payloadLength - 1; |
There was a problem hiding this comment.
nit: the suggestion from https://github.com/netty/netty/pull/5614/files#r73321879 would avoid the need for the extra operation (this is the same as sourcePrefixLength / Byte.SIZE).
normanmaurer
force-pushed
the
dns_rcs
branch
2 times, most recently
from
c83e518 to
797e8c1
Compare
| try { | ||
| // Same as: | ||
| // # /.bind-9.9.3-edns/bin/dig @ns1.google.com www.google.es +client=157.88.0.0/24 | ||
| Future<List<InetAddress>> future = resolver.resolveAll("www.google.es", |
There was a problem hiding this comment.
can we avoid depending on external DNS servers for unit tests?
There was a problem hiding this comment.
@Scottmitch that is why it is marked as @Ignore. There is currently no known java based dns server that support it and so will allow us to test it like this. I put it mainly here as an example how to use it. The encoder itself is tested in DefaultDnsRecordEncoderTest.
There was a problem hiding this comment.
thanks for clarifying ... @Ignore should do the trick for now.
|
@Scottmitch PTAL again |
| throw new Error(); | ||
| } | ||
| } | ||
| @Override |
|
@Scottmitch addressed comments |
| return removed; | ||
| } | ||
|
|
||
| private static boolean emptyAdditionals(DnsRecord[] additionals) { |
There was a problem hiding this comment.
Consider using varargs for methods or constructors which take an array the last parameter. 
| public byte[] address() { | ||
| return address.clone(); | ||
| } | ||
| } |
|
Mostly style nits and validation stuff. Great job! |
|
@trustin I think I addressed everything... PTAL |
Motivation: RFC7871 defines an extension which allows to request responses for a given subset. Modifications: - Add DnsOptPseudoRrRecord which can act as base class for extensions based on EDNS(0) as defined in RFC6891 - Add DnsOptEcsRecord to support the Client Subnet in DNS Queries extension - Add tests Result: Client Subnet in DNS Queries extension is now supported.
normanmaurer
force-pushed
the
dns_rcs
branch
from
6d803cc to
087682f
Compare
|
Cherry-picked into 4.1 (dfa3bbb) |
5 participants
Motivation:
RFC7871 defines an extension which allows to request responses for a given subset.
Modifications:
Result:
Client Subnet in DNS Queries extension is now supported.