Commit Graph

39 Commits

Author SHA1 Message Date
Norman Maurer
1a53df1031 Detect truncated responses caused by EDNS0 and MTU miss-match (#9468)
Motivation:

It is possible that the user uses a too big EDNS0 setting for the MTU and so we may receive a truncated datagram packet. In this case we should try to detect this and retry via TCP if possible

Modifications:

- Fix detecting of incomplete records
- Mark response as truncated if we did not consume the whole packet
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9365
2019-08-17 09:58:40 +02:00
秦世成
ced1d5b751 Pre-decompressed DNS record RData that may contain compression pointers (#9311)
Motivation:

When decoding DnsRecord, if the record contains compression pointers, and not all compression pointers are decompressed, but part of the pointers are decompressed. Then when encoding the record, the compressed pointer will point to the wrong location, resulting in bad label problem.

Modification:

Pre-decompressed record RData that may contain compression pointers.

Result:

Fixes #8962
2019-07-02 19:39:21 +02:00
Norman Maurer
211dde4e08 Add support for TCP fallback when we receive a truncated DnsResponse (#9139)
Motivation:

Sometimes DNS responses can be very large which mean they will not fit in a UDP packet. When this is happening the DNS server will set the TC flag (truncated flag) to tell the resolver that the response was truncated. When a truncated response was received we should allow to retry via TCP and use the received response (if possible) as a replacement for the truncated one.

See https://tools.ietf.org/html/rfc7766.

Modifications:

- Add support for TCP fallback by allow to specify a socketChannelFactory / socketChannelType on the DnsNameResolverBuilder. If this is set to something different then null we will try to fallback to TCP.
- Add decoder / encoder for TCP
- Add unit tests

Result:

Support for TCP fallback as defined by https://tools.ietf.org/html/rfc7766 when using DnsNameResolver.
2019-05-17 14:47:10 +02:00
田欧
db79983874 use checkPositive/checkPositiveOrZero (#8835)
Motivation:

We can replace some "hand-rolled" integer checks with our own static utility method to simplify the code.

Modifications:

Use methods provided by `ObjectUtil`.

Result:

Cleaner code and less duplication
2019-02-04 15:55:07 +01:00
田欧
e8efcd82a8 migrate java8: use requireNonNull (#8840)
Motivation:

We can just use Objects.requireNonNull(...) as a replacement for ObjectUtil.checkNotNull(....)

Modifications:

- Use Objects.requireNonNull(...)

Result:

Less code to maintain.
2019-02-04 10:32:25 +01:00
田欧
9d62deeb6f Java 8 migration: Use diamond operator (#8749)
Motivation:

We can use the diamond operator these days.

Modification:

Use diamond operator whenever possible.

Result:

More modern code and less boiler-plate.
2019-01-22 16:07:26 +01:00
Thomas Devanneaux
32746c53c1 EDNS Client Subnet is not encoded correctly when source prefix length is not a multiple of 8.
Motivation:
When the ECS source prefix length is not a mutiple of 8, the last byte the address inside the
ECS OPT record is not padded properly.

Modifications:
DefaultDnsRecordEncoder.padWithZeros(...) was modified to add padding from the least
significant bits.

Result:
ECS encoding bug fixed.
2017-12-01 08:14:13 +01:00
Nikolay Fedorovskikh
0692bf1b6a fix the typos 2017-04-20 04:56:09 +02:00
Norman Maurer
c2f4daa739 Fix false-positives when using ResourceLeakDetector.
Motivation:

We need to ensure the tracked object can not be GC'ed before ResourceLeak.close() is called as otherwise we may get false-positives reported by the ResourceLeakDetector. This can happen as the JIT / GC may be able to figure out that we do not need the tracked object anymore and so already enqueue it for collection before we actually get a chance to close the enclosing ResourceLeak.

Modifications:

- Add ResourceLeakTracker and deprecate the old ResourceLeak
- Fix some javadocs to correctly release buffers.
- Add a unit test for ResourceLeakDetector that shows that ResourceLeakTracker has not the problems.

Result:

No more false-positives reported by ResourceLeakDetector when ResourceLeakDetector.track(...) is used.
2016-12-04 09:01:39 +01:00
Scott Mitchell
cfa5b85075 DefaultDnsRecordDecoder compression and index decode bug
Motivation:
8cf90f0512 switch a duplicate opreration to a slice operation. Typically this would be fine but DNS supports a compression (https://www.ietf.org/rfc/rfc1035 4.1.4. Message compression) where the payload contains absolute indexes which refer back to previously referenced content. Using a slice will break the ability for the indexes in the payload to correctly self reference to the index of the originial payload, and thus decoding may fail.

Modifications:
- Use duplicate instead of slice so DNS message compression and index references are preserved.

Result:
Fixes DefaultDnsRecordDecoder regression
2016-10-20 09:00:55 -07:00
radai-rosenblatt
15ac6c4a1f Clean-up unused imports
Motivation:

the build doesnt seem to enforce this, so they piled up

Modifications:

removed unused import lines

Result:

less unused imports

Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
2016-09-30 09:08:50 +02:00
Norman Maurer
dfa3bbbf00 Add support for Client Subnet in DNS Queries (RFC7871)
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.
2016-09-06 07:16:57 +02:00
Norman Maurer
8cf90f0512 [#5760] Do not change writerIndex when decode DnsPtrRecord
Motivation:

We need to not change the original writerIndex when decode a DnsPtrRecord as otherwise we will not be able to decode other records that follow it.

Modifications:

Slice the data out and so not increase the writerIndex.

Result:

No more problems when decoding.
2016-09-01 08:26:52 +02:00
Dmitriy Dumanskiy
a80ea46b8e Removed custom split method as it is not effective anymore. 2016-08-01 21:49:33 +02:00
agonigberg
3288cacf8d Pluggable resource leak detector
Allow users of Netty to plug in their own leak detector for the purpose
of instrumentation.

Motivation:

We are rolling out a large Netty deployment and want to be able to
track the amount of leaks we're seeing in production via custom
instrumentation. In order to achieve this today, I had to plug in a
custom `ByteBufAllocator` into the bootstrap and have it initialize a
custom `ResourceLeakDetector`. Due to these classes mostly being marked
`final` or having private or static methods, a lot of the code had to
be copy-pasted and it's quite ugly.

Modifications:

* I've added a static loader method for the `ResourceLeakDetector` in
`AbstractByteBuf` that tries to instantiate the class passed in via the
`-Dio.netty.customResourceLeakDetector`, otherwise falling back to the
default one.
* I've modified `ResourceLeakDetector` to be non-final and to have the
reporting broken out in to methods that can be overridden.

Result:

You can instrument leaks in your application by just adding something
like the following:

```java
public class InstrumentedResourceLeakDetector<T> extends
ResourceLeakDetector<T> {

    @Monitor("InstanceLeakCounter")
    private final AtomicInteger instancesLeakCounter;

    @Monitor("LeakCounter")
    private final AtomicInteger leakCounter;

    public InstrumentedResourceLeakDetector(Class<T> resource) {
        super(resource);
        this.instancesLeakCounter = new AtomicInteger();
        this.leakCounter = new AtomicInteger();
    }

    @Override
    protected void reportTracedLeak(String records) {
        super.reportTracedLeak(records);
        leakCounter.incrementAndGet();
    }

    @Override
    protected void reportUntracedLeak() {
        super.reportUntracedLeak();
        leakCounter.incrementAndGet();
    }

    @Override
    protected void reportInstancesLeak() {
        super.reportInstancesLeak();
        instancesLeakCounter.incrementAndGet();
    }
}
```
2016-06-20 11:14:44 +02:00
Norman Maurer
c7a0a0f325 [#5391] DnsNameResolver does not resolve property A+CNAME answer
Motivation:

The current DnsNameResolver fails to resolve an A+CNAME answer. For example:

dig moose.rmq.cloudamqp.com

...
;; ANSWER SECTION:
moose.rmq.cloudamqp.com. 1800   IN  CNAME   ec2-54-152-221-139.compute-1.amazonaws.com.
ec2-54-152-221-139.compute-1.amazonaws.com. 583612 IN A 54.152.221.139
...

The resolver constructs a map of cnames but forgets the trailing "." in the values which lead to not resolve the A record.

Modifications:

Reuse the code of DefaltDnsRecordDecoder which correctly handles the trailing dot.

Result:

Correctly resolve.
2016-06-20 07:13:00 +02:00
Trustin Lee
3a9f472161 Make retained derived buffers recyclable
Related: #4333 #4421 #5128

Motivation:

slice(), duplicate() and readSlice() currently create a non-recyclable
derived buffer instance. Under heavy load, an application that creates a
lot of derived buffers can put the garbage collector under pressure.

Modifications:

- Add the following methods which creates a non-recyclable derived buffer
  - retainedSlice()
  - retainedDuplicate()
  - readRetainedSlice()
- Add the new recyclable derived buffer implementations, which has its
  own reference count value
- Add ByteBufHolder.retainedDuplicate()
- Add ByteBufHolder.replace(ByteBuf) so that..
  - a user can replace the content of the holder in a consistent way
  - copy/duplicate/retainedDuplicate() can delegate the holder
    construction to replace(ByteBuf)
- Use retainedDuplicate() and retainedSlice() wherever possible
- Miscellaneous:
  - Rename DuplicateByteBufTest to DuplicatedByteBufTest (missing 'D')
  - Make ReplayingDecoderByteBuf.reject() return an exception instead of
    throwing it so that its callers don't need to add dummy return
    statement

Result:

Derived buffers are now recycled when created via retainedSlice() and
retainedDuplicate() and derived from a pooled buffer
2016-05-17 11:16:13 +02:00
Norman Maurer
9229ed98e2 [#5088] Add annotation which marks packages/interfaces/classes as unstable
Motivation:

Some codecs should be considered unstable as these are relative new. For this purpose we should introduce an annotation which these codecs should us to be marked as unstable in terms of API.

Modifications:

- Add UnstableApi annotation and use it on codecs that are not stable
- Move http2.hpack to http2.internal.hpack as it is internal.

Result:

Better document unstable APIs.
2016-05-09 15:16:35 +02:00
Trustin Lee
516e4933c4 Remove an unused import 2016-04-02 01:39:47 -04:00
Trustin Lee
4b38b72a0d Fix incorrect name encoding/decoding in DNS records
Motivation:

- The decoded name should always end with a dot (.), but we currently
  strip it, which is incorrect.
  - (O) 0 -> "."
  - (X) 0 -> ""
  - (O) 5 netty 2 io 0 -> "netty.io."
  - (X) 5 netty 2 io 0 -> "netty.io"
- The encoded name should end with a null-label, which is a label whose
  length is 0, but we currently append an extra NUL, causing FORMERR(1)
  on a strict DNS server:
  - (O) . -> 0
  - (X) . -> 0 0
  - (O) netty.io. -> 5 netty 2 io 0
  - (X) netty.io. -> 5 netty 2 io 0 0

Modifications:

- Make sure to append '.' when decoding a name.
- Improve index checks so that the decoder can raise
  CorruptFrameException instead of IIOBE
- Do not encode extra NUL
- Add more tests

Result:

Robustness and correctness
2016-04-01 22:18:33 +02:00
Norman Maurer
6bf7e24389 [#5014] Correctly encode / decode zero-length names when encoding DnsRecords.
Motivation:

Zero-length names needs to be "prefixed" by the length as well when encoded into a ByteBuf. Also some servers not correctly prefix these so we should ensure we can workaround this and even decode in such case.

Modifications:

- Always encode the length of the name into the ByteBuf even if its zero-length.
- If there are no readable bytes for the name just asume its an empty name to workaround dns servers that not fully respect the RFC.

Result:

Correctly encode zero-length names and be able to decode empty names even when the rfc is not strictly followed.
2016-03-24 13:51:04 +01:00
Stephane Landelle
3d115349b5 Fix type inference w/ JDK8
Motivation:

Compile crash w/ JDK8:

```
[ERROR]
/Users/slandelle/Documents/dev/workspaces/workspace-ahc2/async-http-clie
nt-project/netty-bp/codec-dns/src/main/java/io/netty/handler/codec/dns/D
nsMessageUtil.java:[176,16] reference to append is ambiguous
  both method append(java.lang.String) in java.lang.StringBuilder and
method append(java.lang.StringBuffer) in java.lang.StringBuilder match
```

Modification:

Force type explicitly

Result:

Class compile w/ JDK8
2016-03-23 18:06:38 +01:00
Norman Maurer
48506f5b05 [#4993] Correctly handle trailing dot in DNS requests and responses for the hostname.
Motivation:

We need to handle the trailing dot in the correct manner when creating DNS questions and responses.

Modifications:

- Add a trailing dot if not given to the hostname when construct a AbstractDnsRecord (this is the same as dig does).

Result:

Correctly handle trailing dots.
2016-03-22 12:30:46 +01:00
Norman Maurer
c3c1b4a6d2 [#4937] [#4935] Correctly valid domain name length and convert to ASCII.
Motivation:

Domain name labels must be converted to ASCII and not be longer then 63 chars.

Modifications:

Correctly convert to ASCII which also will enforce the 63 chars length.

Result:

Correctly guard against invalid input.
2016-03-16 11:55:19 +01:00
Fabian Lange
58c63b7888 Add support for directly decoding/encoding DNS PTR Records
Motivation:

The current implementation will provide a DnsRawRecord, which, while
containing the host name it resolves to, would require the user to
decode the name using the decode method currently private to
DefaultDnsRecordDecoder, which in fact means copying it.

Modifications:

Introduce DnsPtrRecord, which is a specialization of DnsRecord which
provides a decoded host name.

Result:

PTR Records are much easier to work with, as the name is decoded already.
2016-03-08 14:59:46 +01:00
Yeti Sno
b9ae48589b Make codec-dns can support build a dns server, reply answer from client.
Motivation:

codec-dns has great function to solve dns packet, but only make a query, not answer query from other client.

i make a change of add two classes to fill last pieces of map, finish the server function.

Modifications:

in this change, add two classes of DatagramDnsQueryDecoder and DatagramDnsResponseEncoder to handle client query, reply answer.

Result:

nothing code change after this commit, except two new classes.
2016-01-18 16:41:44 +09:00
Trustin Lee
fdfe3149ba Provide more control over DnsNameResolver.query() / Add NameResolver.resolveAll()
Related issues:
- #3971
- #3973
- #3976
- #4035

Motivation:

1. Previously, DnsNameResolver.query() retried the request query by its
own. It prevents a user from deciding when to retry or stop. It is also
impossible to get the response object whose code is not NOERROR.

2. NameResolver does not have an operation that resolves a host name
into multiple addresses, like InetAddress.getAllByName()

Modifications:

- Changes related with DnsNameResolver.query()
  - Make query() not retry
    - Move the retry logic to DnsNameResolver.resolve() instead.
  - Make query() fail the promise only when I/O error occurred or it
    failed to get a response
  - Add DnsNameResolverException and use it when query() fails so that
    the resolver can give more information about the failure
  - query() does not cache anymore.

- Changes related with NameResolver.resolveAll()
  - Add NameResolver.resolveAll()
  - Add SimpleNameResolver.doResolveAll()

- Changes related with DnsNameResolver.resolve() and resolveAll()
  - Make DnsNameResolveContext abstract so that DnsNameResolver can
    decide to get single or multiple addresses from it
  - Re-implement cache so that the cache works for resolve() and
    resolveAll()
  - Add 'traceEnabled' property to enable/disable trace information

- Miscellaneous changes
  - Use ObjectUtil.checkNotNull() wherever possible
  - Add InternetProtocolFamily.addressType() to remove repetitive
    switch-case blocks in DnsNameResolver(Context)
  - Do not raise an exception when decoding a truncated DNS response

Result:

- Full control over query()
- A user can now retrieve all addresses via (Dns)NameResolver.resolveAll()
- DNS cache works only for resolve() and resolveAll() now.
2015-08-18 17:40:13 +09:00
Trustin Lee
5e0ee6c095 Fix missing ResourceLeak.close() in AbstractDnsMessage
Motivation:

ResourceLeak.close() must be called when a reference-counted resource is
deallocated, but AbstractDnsMessage.deallocate() forgot to call it.

Modifications:

Call ResourceLeak.close() for the tracked AbstractDnsMessage instances

Result:

Fix the false resource leak warnings
2015-05-04 12:05:59 +09:00
Trustin Lee
63a02fc04e Revamp DNS codec
Motivation:

There are various known issues in netty-codec-dns:

- Message types are not interfaces, which can make it difficult for a
  user to implement his/her own message implementation.
- Some class names and field names do not match with the terms in the
  RFC.
- The support for decoding a DNS record was limited. A user had to
  encode and decode by him/herself.
- The separation of DnsHeader from DnsMessage was unnecessary, although
  it is fine conceptually.
- Buffer leak caused by DnsMessage was difficult to analyze, because the
  leak detector tracks down the underlying ByteBuf rather than the
  DnsMessage itself.
- DnsMessage assumes DNS-over-UDP.
- To send an EDNS message, a user have to create a new DNS record class
  instance unnecessarily.

Modifications:

- Make all message types interfaces and add default implementations
- Rename some classes, properties, and constants to match the RFCs
  - DnsResource -> DnsRecord
  - DnsType -> DnsRecordType
  - and many more
- Remove DnsClass and use an integer to support EDNS better
- Add DnsRecordEncoder/DnsRecordDecoder and their default
  implementations
  - DnsRecord does not require RDATA to be ByteBuf anymore.
  - Add DnsRawRecord as the catch-all record type
- Merge DnsHeader into DnsMessage
- Make ResourceLeakDetector track AbstractDnsMessage
- Remove DnsMessage.sender/recipient properties
  - Wrap DnsMessage with AddressedEnvelope
  - Add DatagramDnsQuest and DatagramDnsResponse for ease of use
  - Rename DnsQueryEncoder to DatagramDnsQueryEncoder
  - Rename DnsResponseDecoder to DatagramDnsResponseDecoder
- Miscellaneous changes
  - Add StringUtil.TAB

Result:

- Cleaner APi
- Can support DNS-over-TCP more easily in the future
- Reduced memory footprint in the default DnsQuery/Response
  implementations
- Better leak tracking for DnsMessages
- Possibility to introduce new DnsRecord types in the future and provide
  full record encoder/decoder implementation.
- No unnecessary instantiation for an EDNS pseudo resource record
2015-05-01 11:33:16 +09:00
Trustin Lee
ab2e80fbb1 Add EDNS support to DnsQueryEncoder
Motivation:

DnsQueryEncoder does not encode the 'additional resources' section at all, which contains the pseudo-RR as defined in RFC 2671.

Modifications:

- Modify DnsQueryEncoder to encode the additional resources
- Fix a bug in DnsQueryEncoder where an empty name is encoded incorrectly

Result:

A user can send an EDNS query.
2014-10-16 17:05:08 +09:00
Norman Maurer
74dd295e59 Fix buffer leaks in DnsResponseDecoder and DnsResponseDecoderTest
Motivation:

There were two buffer leaks in the codec-dns.

Modifications:

- Fix buffer leak in DnsResponseTest.readResponseTest()
- Correctly release DnsResources on Exception

Result:

No more buffer leaks in the codec-dns module.
2014-08-04 14:06:45 +02:00
Norman Maurer
e09d2f32fb [#2708] DnsResource.duplicate() should return DnsResource and not ByteBufHolder
Motivation:

DnsResource.duplicate() should return DnsResource and not ByteBufHolder

Modifications:

Change return type from ByteBufHolder to DnsResource

Result:

No need to cast to the correct type when using duplicate()
2014-07-28 04:26:09 -07:00
Trustin Lee
4bde044957 Overall cleanup of codec-dns
Related issue: #2688

- DnsClass and DnsType
  - Make DnsClass and DnsType implement Comparable
  - Optimize the message generation of IllegalArgumentException,
    by pre-populating the list of the expected parameters
  - Move the static methods up
  - Relax the validation rule of DnsClass so that a user can define a
    CLASS which is not listed in the RFC 1035
  - valueOf(int) does not throw IllegalArgumentException anymore as long
    as the specified value is an unsigned short.
  - Rename create() and forName() to valueOf() so that they look like a
    real enum
  - Rename type() and clazz() to intValue() so that they conform to our
    naming convention
- Add missing null checks in DnsEntry
2014-07-23 14:40:52 -07:00
Tim Boudreau
9734170b7d Use value types for class and type in DNS entries to make them immune to parameter order bugs
Motivation:

DNS class and type were represented as integers rather than an enum or a
similar dedicated value type.  This can be a potential source of a
parameter order bug which might be difficult to track down.

Modifications:

Add DnsClass and DnsType to replace integer parameters

Result:

Type safety and less error-proneness
2014-07-23 14:40:52 -07:00
Trustin Lee
923a0e71ac Raise a meaningful exception instead of NPE
Motivation:

When decoding the NAME field in a DNS Resource Record, DnsResponseDecoder
can raise a NullPointerException if the NAME field contains a loop.

Modification:

Instead of raising an NPE, raise CorruptedFrameException so that the
exception itself has meaning.

Result:

Less confusing when a malformed DNS RR is received
2014-07-21 16:51:38 -07:00
Trustin Lee
ae9963a40c Fix NPE while decoding authority section of a DNS response
Motivation:

NullPointerException is raised when a DNS response conrains a resource
record whose NAME is empty, which is the case for the authority section.

Modification:

Allow an empty name for DnsEntry. Only disallow an empty name for
DnsQuestion.

Result:

Fixes #2686
2014-07-21 16:40:26 -07:00
Adam
702ebbc19b Don't spin from malformed dns packets containing loops
Motivation:

DNS packets that contain pointers in a loop will cause
DnsResponseDecoder.readName() to infinite loop.

Modifications:

Fixed DnsResponseDecoder.readName() to detect when packets have loops
and return null instead.

Result:

It is no longer possible to cause Netty to infinite loop by sending it malformed
DNS packets with a loop.
2014-07-20 14:09:16 +02:00
Trustin Lee
d0912f2709 Fix most inspector warnings
Motivation:

It's good to minimize potentially broken windows.

Modifications:

Fix most inspector warnings from our profile
Update IntObjectHashMap

Result:

Cleaner code
2014-07-02 19:55:07 +09:00
Norman Maurer
e3c76ec106 DNS codec for Netty which is based on the work of [#1622].
Motivation:
As part of GSOC 2013 we had @mbakkar working on a DNS codec but did not integrate it yet as it needs some cleanup. This commit is based on @mbakkar's work and provide the codec for DNS.

Modifications:
Add DNS codec

Result:
Reusable DNS codec will be included in netty.

This PR also includes a AsynchronousDnsResolver which allows to resolve DNS entries in a non blocking way by make use
of the dns codec and netty transport itself.
2014-06-10 09:57:06 +02:00