Commit Graph

9126 Commits

Author SHA1 Message Date
Nikolay Fedorovskikh
587afddb27 Fixes NPE in ClientCookieDecoder
Motivation:
NPE in `ClientCookieDecoder` if cookie starts with comma.

Modifications:
Check `cookieBuilder` for `null` in the return.

Result:
No fails NPE on invalid cookies.
2018-04-05 19:44:08 +02:00
ikurovsky
c58069f284 Better handling of streaming JSON data in JsonObjectDecoder (#7821)
Motivation:

When the JsonObjectDecoder determines that the incoming buffer had some data discarded, it resets the internal index to readerIndex and attempts to adjust the state which does not correctly work for streams of JSON objects.

Modifications:

Reset the internal index to the value considering the previous reads.

Result:

JsonObjectDecoder correctly handles streams of both JSON objects and arrays with no state adjustments or repeatable reads.
2018-04-05 07:58:14 +02:00
Dave Moten
0f4001d598 add task before starting thread in SingleThreadEventExecutor.execute (#7841)
Motivation:

Minor performance optimisation that prevents thread from blocking due to task not having been added to queue. Discussed #7815.

Modification:

add task to the queue before starting the thread.

Result:

No additional tests.
2018-04-05 07:57:21 +02:00
root
0a61f055f5 [maven-release-plugin] prepare for next development iteration 2018-04-04 10:44:46 +00:00
root
8c549bad38 [maven-release-plugin] prepare release netty-4.1.23.Final 2018-04-04 10:44:15 +00:00
Nikolay Fedorovskikh
a95fd91bc6 Don't check accessible in the #capacity method (#7830)
Motivation:
The `#ensureAccessible` method in `UnpooledHeapByteBuf#capacity` used
to prevent NPE if buffer is released and `array` is `null`. In all
other implementations of `ByteBuf` the accessible is not checked by
`capacity` method. We can assign an empty array to `array`
in the `deallocate` and don't worry about NPE in the `#capacity`.
This will help reduce the number of repeated calls of the
`#ensureAccessible` in many operations with `UnpooledHeapByteBuf`.

Modifications:
1. Remove `#ensureAccessible` call from `UnpooledHeapByteBuf#capacity`.
Use the `EmptyArrays#EMPTY_BYTES` instead of `null` in `#deallocate`.

2. Fix access checks in `AbstractUnsafeSwappedByteBuf` and
`AbstractByteBuf#slice` that relied on `#ensureAccessible`
in `UnpooledHeapByteBuf#capacity`. This was found by unit tests.

Result:
Less double calls of `#ensureAccessible` for `UnpooledHeapByteBuf`.
2018-04-03 21:35:02 +02:00
Norman Maurer
ee9057ad99
CorsHandler.write(...) should not cause a flush. (#7839)
Motivation:

Unnecessary flushes reduce the amount of flush coalescing that can happen at higher levels and thus can increase number of packets (because of TCP_NODELAY) and lower throughput (due to syscalls, TLS frames, etc)

Modifications:

Replace writeAndFlush(...) with write(...)

Result:

Fixes https://github.com/netty/netty/issues/7837.
2018-04-03 21:11:51 +02:00
Norman Maurer
473f6a6edb
SctpMessageCompletionHandler may leak ByteBuf for fragmented messages. (#7832)
Motivation:

SctpMessageCompletionHandler stores fragments in a Map but not release the stored ByteBuf when the handler is removed.

Modifications:

Release all buffers that are still in the Map when handlerRemoved(...) is called.

Result:

No more leaks for fragemented messages.
2018-04-02 21:37:03 +02:00
Bryce Anderson
741602050f Don't replace all 'connection' headers when sending h2c upgrade request (#7824)
Motivation:

There may be meaningful 'connection' headers that exist on a request
that is used to attempt a HTTP/1.x upgrade request that will be
clobbered.

Modifications:

HttpClientUpgradeHandler uses the `HttpHeaders.add` instead of
`HttpHeaders.set` when adding the 'upgrade' field.

Result:

Fixes #7823, existing 'connection' headers are preserved.
2018-04-01 19:59:30 +02:00
Scott Mitchell
602ee5444d NetUtil valid IP methods to accept CharSequence (#7827)
* NetUtil valid IP methods to accept CharSequence

Motivation:
NetUtil has methods to determine if a String is a valid IP address. These methods don't rely upon String specific methods and can use CharSequence instead.

Modifications:
- Use CharSequence instead of String for the IP validator methods.
- Avoid object allocation in AsciiString#indexOf(char,int) and reduce
byte code

Result:
No more copy operation required if a CharSequence exists.
2018-04-01 08:39:43 +02:00
Scott Mitchell
9d51a40df0 Update NetUtilBenchmark (#7826)
Motivation:
NetUtilBenchmark is using out of date data, throws an exception in the benchmark, and allocates a Set on each run.

Modifications:
- Update the benchmark and reduce each run's overhead

Result:
NetUtilBenchmark is updated.
2018-03-31 08:27:08 +02:00
Nicolae Mihalache
8d78893a76 Avoid writing two times the same message in case channelWritabilityChanged event is called during a write.
Motivation:

ChunkedWriteHandler.doFlush is called twice from the same write if the channelWritabilityChanged event is invoked during the write. The buffer is already written so no extra data is sent on the socket but it causes the "promise already done" exception to be thrown.
This error happens only when the message is not ChunkedInput.

Modification:
Clear out the currentWrite reference before the ctx.write call, such that next time when the method is invoked the same object is not used twice.

Result:

Fixes #7819
2018-03-30 19:32:08 +02:00
Trustin Lee
cd4594d292 Add DnsNameResolver.resolveAll(DnsQuestion) (#7803)
* Add DnsNameResolver.resolveAll(DnsQuestion)

Motivation:

A user is currently expected to use DnsNameResolver.query() when he or
she wants to look up the full DNS records rather than just InetAddres.

However, query() only performs a single query. It does not handle
/etc/hosts file, redirection, CNAMEs or multiple name servers.

As a result, such a user has to duplicate all the logic in
DnsNameResolverContext.

Modifications:

- Refactor DnsNameResolverContext so that it can send queries for
  arbitrary record types.
  - Rename DnsNameResolverContext to DnsResolveContext
  - Add DnsAddressResolveContext which extends DnsResolveContext for
    A/AAAA lookup
  - Add DnsRecordResolveContext which extends DnsResolveContext for
    arbitrary lookup
- Add DnsNameResolverContext.resolveAll(DnsQuestion) and its variants
- Change DnsNameResolverContext.resolve() delegates the resolve request
  to resolveAll() for simplicity
- Move the code that decodes A/AAAA record content to DnsAddressDecoder

Result:

- Fixes #7795
- A user does not have to duplicate DnsNameResolverContext in his or her
  own code to implement the usual DNS resolver behavior.
2018-03-29 22:01:25 +02:00
Norman Maurer
965734a1eb
Limit the number of bytes to use to copy the content of a direct buffer to an Outputstream (#7813)
Motivation:

Currently copying a direct ByteBuf copies it fully into the heap before writing it to an output stream.
The can result in huge memory usage on the heap.

Modification:

copy the bytebuf contents via an 8k buffer into the output stream

Result:

Fixes #7804
2018-03-29 12:49:27 +02:00
Scott Mitchell
ed0668384b NIO read spin event loop spin when half closed (#7801)
Motivation:
AbstractNioByteChannel will detect that the remote end of the socket has
been closed and propagate a user event through the pipeline. However if
the user has auto read on, or calls read again, we may propagate the
same user events again. If the underlying transport continuously
notifies us that there is read activity this will happen in a spin loop
which consumes unnecessary CPU.

Modifications:
- AbstractNioByteChannel's unsafe read() should check if the input side
of the socket has been shutdown before processing the event. This is
consistent with EPOLL and KQUEUE transports.
- add unit test with @normanmaurer's help, and make transports consistent with respect to user events

Result:
No more read spin loop in NIO when the channel is half closed.
2018-03-28 20:02:57 +02:00
Bryce Anderson
b309271e49 HttpServerUpgradeHandler shouldn't wait for flush to reshape pipeline
Motivation:

There is a race between both flushing the upgrade response and receiving
more data before the flush ChannelPromise can fire and reshape the
pipeline. Since We have already committed to an upgrade by writing the
upgrade response, we need to be immediately prepared for handling the
next protocol.

Modifications:

The pipeline reshaping logic in HttpServerUpgradeHandler has been moved
out of the ChannelFutureListener attached to the write of the upgrade
response and happens immediately after the writeAndFlush call, but
before the method returns.

Result:

The pipeline is no longer subject to receiving more data before the
pipeline has been reformed.
2018-03-28 19:54:30 +02:00
Norman Maurer
5ebee9426f Update to conscrypt 1.0.1
Motivation:

We should use the latest conscrypt release.

Modifications:

Update to 1.0.1

Result:

Use latest conscrypt
2018-03-27 19:09:54 +02:00
Norman Maurer
0bb042158c Update to netty-tcnative 2.0.8.Final
Motivation:

netty-tcnative 2.0.8.Final was released.

Modifications:

Update to latest netty-tcnative release.

Result:

Use latest release of tcnative
2018-03-27 19:09:34 +02:00
Dmitriy Dumanskiy
62d8a5e9d2 Add removeIfExists() method to DefaultChannelPipeline
Motivation:

Sometimes it is very convenient to remove the handler from pipeline without throwing the exception in case those handler doesn't exist in the pipeline.

Modification:

Added 3 overloaded methods to DefaultChannelPipeline, but not added to ChannelHandler due to back compatibility.

Result:

Fixes #7662
2018-03-27 09:48:52 +02:00
Marian Seitner
c75bc1f25b Support Redis inline commands
Motivation:
The RESP protocol implementation lacked inline command
support.

Modifications:
Added logic to decode and encode inline commands.

Result:
Inline commands are supported. Fixes #7686.
2018-03-27 09:46:20 +02:00
Norman Maurer
2e92a2f5cd Ensure we not schedule multiple timeouts for close notify
Motivation:

We should only schedule one timeout to wait for the close notify to be done.

Modifications:

Keep track of if we already scheduled a timeout for close notify and if so not schedule another one.

Result:

No duplicated timeouts.
2018-03-27 09:43:46 +02:00
Stephane Landelle
d60cd0231d HttpProxyHandler generates invalid CONNECT url and Host header when address is resolved
Motivation:

HttpProxyHandler uses `NetUtil#toSocketAddressString` to compute
CONNECT url and Host header.

The url is correct when the address is unresolved, as
`NetUtil#toSocketAddressString` will then use
`getHoststring`/`getHostname`. If the address is already resolved, the
url will be based on the IP instead of the hostname.

There’s an additional minor issue with the Host header: default port
443 should be omitted.

Modifications:

* Introduce NetUtil#getHostname
* Introduce HttpUtil#formatHostnameForHttp to format an
InetSocketAddress to
HTTP format
* Change url computation to favor hostname instead of IP
* Introduce HttpProxyHandler ignoreDefaultPortsInConnectHostHeader
parameter to ignore 80 and 443 ports in Host header

Result:

HttpProxyHandler performs properly when connecting to a resolved address
2018-03-27 09:43:11 +02:00
Norman Maurer
fc3b145cbb Correctly handle non IOException during read in NioServerSocketChannel
Motivation:

Our code was not correct in AbstractNioMessageChannel.closeOnReadError(....) which lead to the situation that we always tried to continue reading no matter what exception was thrown when using the NioServerSocketChannel. Also even on an IOException we should check if the Channel itself is still active or not and if not stop reading.

Modifications:

Fix closeOnReadError impl and added test.

Result:

Correctly stop reading on NioServerSocketChannel when error happens during read.
2018-03-25 17:31:59 +02:00
Norman Maurer
8189399e9d Ignore EINTR on close(...) as there is nothing sane we can do.
Motivation:

If close(...) reports EINTR there is nothing sane we can do so it makes no sense to even report it. See also:

https://github.com/apple/swift-nio/pull/217

Modifications:

Just ignore EINTR when calling close(...)

Result:

Less noise in the logs.
2018-03-23 07:39:17 +01:00
Norman Maurer
40af10b782 Skip NPN tests when libressl 2.6.1+ is used.
Motivation:

LibreSSL removed support for NPN in its 2.6.1+ releases.

Modifications:

Skip NPN tests in libressl 2.6.1+

Result:

Be able to run netty tests against libressl 2.6.1+ as well.
2018-03-22 08:30:49 +01:00
Norman Maurer
6e6cfa0604 Add tests for EmptyHeaders
Motivation:

6e5fd9311f fixed a bug in EmptyHeaders which was never noticed before because we had no tests.

Modifications:

Add tests for EmptyHeaders.

Result:

EmptyHeaders is tested now.
2018-03-20 15:59:08 +01:00
Scott Mitchell
6e5fd9311f EmptyHeaders get with default value returns null
Motivation:
EmptyHeaders#get with a default value argument returns null. It should never return null, and instead it should return the default value.

Modifications:
- EmptyHeaders#get with a default value should return that default value

Result:
More correct implementation of the Headers API.
2018-03-19 17:54:26 +01:00
Norman Maurer
352e36a179 Remove code duplication in ChunkedWriteHandler
Motivation:

We had some code duplication in ChunkedWriteHandler.

Modifications:

Factor out duplicated code into private methods and reuse it.

Result:

Less code duplication.
2018-03-19 09:14:07 +01:00
Norman Maurer
2c90b6235d Correctly include the stream id when convert from Http2HeadersFrame to HttpMessage
Motivation:

We did not correctly set the stream id in the headers of HttpMessage when converting a Http2HeadersFrame. This is based on https://github.com/netty/netty/pull/7778 so thanks to @jprante.

Modifications:

- Correctly set the id when possible in the header.
- Add test case

Result:

Correctly include stream id.
2018-03-17 09:46:01 +01:00
Norman Maurer
0adccfdb50 Simplify DefaultChannelGroup.contains(...) and so remove one instanceof check.
Motivation:

DefaultChannelGroup.contains(...) did one more instanceof check then needed.

Modifications:

Simplify contains(...) and remove one instanceof check.

Result:

Simplier and cheaper implementation.
2018-03-17 09:45:24 +01:00
Norman Maurer
de082bf4c7 Correctly record creation stacktrace in ResourceLeakDetector.
Motivation:

We missed to correctly record the stacktrace of the creation of an ResourceLeak record. This could either have the effect to log the wrote stacktrace for creation or not log a stacktrace at all if the object was dropped on the floor after it was created.

Modifications:

Correctly create a Record on creation of the object.

Result:

Fixes https://github.com/netty/netty/issues/7781.
2018-03-16 08:24:18 +01:00
Alexey Kachayev
b0823761f4 PendingWriteQueue to handle write operations with void future
Motivation:

Right now PendingWriteQueue.removeAndWriteAll collects all promises to
PromiseCombiner instance which sets listener to each given promise throwing
IllegalStateException on VoidChannelPromise which breaks while loop
and "reports" operation as failed (when in fact part of writes might be
actually written).

Modifications:

Check if the promise is not void before adding it to the PromiseCombiner
instance.

Result:

PendingWriteQueue.removeAndWriteAll succesfully writes all pendings
even in case void promise was used.
2018-03-16 08:23:40 +01:00
Norman Maurer
f6251c8256 IovArray.add(...) should check if buffer has memory address.
Motivation:

We currently not check if the buffer has a memory address and just assume this is the case if the nioBufferCount() == 1.

Modifications:

- Check hasMemoryAddress() before trying to access it.
- Add unit case.

Result:

More correct and robust code. Related to [#7752].
2018-03-13 08:51:01 +01:00
Norman Maurer
bd772d127e FixedCompositeByteBuf should allow to access memoryAddress / array when wrap a single buffer.
Motivation:

We should allow to access the memoryAddress / array of the FixedCompositeByteBuf when it only wraps a single ByteBuf. We do the same for CompositeByteBuf.

Modifications:

- Check how many buffers FixedCompositeByteBuf wraps and depending on it delegate the access to the memoryAddress / array
- Add unit tests.

Result:

Fixes [#7752].
2018-03-13 08:50:42 +01:00
Norman Maurer
6eb9674bf5 Replace finalizer() usage in Recycler.WeakOrderQueue with ObjectCleaner usage.
Motivation:

We recently introduced ObjectCleaner which can be used to ensure some cleanup action is done once an object becomes weakable reachable. We should use this in Recycler.WeakOrderQueue to reduce the overhead of using a finalizer() (which will cause the GC to process it two times).

Modifications:

Replace finalizer() usage with ObjectCleaner

Result:

Fixes [#7343]
2018-03-09 18:45:02 -08:00
Norman Maurer
d1055e0665 Add testcase for c11b23bbc1
Motivation:

c11b23bbc1 added a fix for closing the SSLEngine otbound but no test was provided.

Modifications:

Add testcase.

Result:

More tests.
2018-03-06 14:33:54 +09:00
Carl Mastrangelo
c11b23bbc1 Close SSLEngine when connection fails.
Motivation:
When using the JdkSslEngine, the ALPN class is used keep a reference
to the engine.   In the event that the TCP connection fails, the
SSLEngine is not removed from the map, creating a memory leak.

Modification:
Always close the SSLEngine regardless of if the channel became
active.  Also, record the SSLEngine was closed in all places.

Result:
Fixes: https://github.com/grpc/grpc-java/issues/3080
2018-03-04 06:55:51 -08:00
Norman Maurer
bf8cac4939 Workaround SSLEngine.unwrap(...) bug in Android 5.0
Motivation:

Android 5.0 sometimes not correctly update the bytesConsumed of the SSLEngineResult when consuming data from the input ByteBuffer. This will lead to handshake failures.

Modifications:

Add a workaround for Android 5.0

Result:

Be able to use netty on Android 5.0 by fixing https://github.com/netty/netty/issues/7758 .
2018-03-03 15:01:39 -08:00
Norman Maurer
48df2f66b8 HashedWheelTimer.newTimeout(...) may overflow
Motivation:

We dont protect from overflow and so the timer may fire too early if a large timeout is used.

Modifications:

Add overflow guard and a test.

Result:

Fixes https://github.com/netty/netty/issues/7760.
2018-03-03 15:00:47 -08:00
Norman Maurer
0a8e1aaf19 Flush task should not flush messages that were written since last flush attempt.
Motivation:

The flush task is currently using flush() which will have the affect of have the flush traverse the whole ChannelPipeline and also flush messages that were written since we gave up flushing. This is not really correct as we should only continue to flush messages that were flushed at the point in time when the flush task was submitted for execution if the user not explicit call flush() by him/herself.

Modification:

Call *Unsafe.flush0() via the flush task which will only continue flushing messages that were marked as flushed before.

Result:

More correct behaviour when the flush task is used.
2018-03-02 10:09:40 +09:00
kakashiio
12ccd40c5a Correctly throw IndexOutOfBoundsException when writerIndex < readerIndex
Motivation:

If someone invoke writeByte(), markWriterIndex(), readByte() in order first, and then invoke resetWriterIndex() should be throw a IndexOutOfBoundsException to obey the rule that the buffer declared "0 <= readerIndex <= writerIndex <= capacity".

Modification:

Changed the code writerIndex = markedWriterIndex; into writerIndex(markedWriterIndex); to make the check affect

Result:
Throw IndexOutOfBoundsException if any invalid happened in resetWriterIndex.
2018-03-02 10:05:33 +09:00
Francesco Nigro
ed46c4ed00 Copies from read-only heap ByteBuffer to direct ByteBuf can avoid stealth ByteBuf allocation and additional copies
Motivation:

Read-only heap ByteBuffer doesn't expose array: the existent method to perform copies to direct ByteBuf involves the creation of a (maybe pooled) additional heap ByteBuf instance and copy

Modifications:

To avoid stressing the allocator with additional (and stealth) heap ByteBuf allocations is provided a method to perform copies using the (pooled) internal NIO buffer

Result:

Copies from read-only heap ByteBuffer to direct ByteBuf won't create any intermediate ByteBuf
2018-02-27 09:54:21 +09:00
Norman Maurer
a53f716ca1 Call correct super methods in test.
Motivation:

We called the wrong super method in the test and also had a few unused imports.

Modifications:

Fix super method call and cleanup.

Result:

More correct test and cleanup.
2018-02-27 09:52:32 +09:00
teaey
06dcca1dbc When the response exceeds the threshold, it will be compressed
Motivation:

When the response is very small, compression will inflate the response.

Modifications:

Add filed io.netty.handler.codec.http.HttpContentCompressor#compressThreshold that control whether the HTTP response should be compressed.

Result:

Fixes #7660.
2018-02-25 16:43:45 +01:00
Norman Maurer
69582c0b6c [maven-release-plugin] prepare for next development iteration 2018-02-21 12:52:33 +00:00
Norman Maurer
786f35c6c9 [maven-release-plugin] prepare release netty-4.1.22.Final 2018-02-21 12:52:19 +00:00
Scott Mitchell
d2d3e6ef0c
KQueue write filter initial state (#7738)
Motivation:
KQueue implementations current have inconsistent behavior with Epoll implementations with respect to asynchronous sockets and connecting. In the Epoll transport we attempt to connect, if the connect call does not synchornously fail/succeed we set the EPOLLOUT which will be triggered by the kernel if the connection attempt succeeds or an error occurs. The connect API provides no way to asynchronously communicate an error so the Epoll implementation fires a EPOLLOUT event and puts the connect status in getsockopt(SO_ERROR). KQueue provides the same APIs but different behavior. If the EVFILT_WRITE is not enabled and the EVFILT_READ is enabled before connect is called, and there is an error the kernel may fire the EVFILT_READ filter and provide the Connection Refused error via read(). This is even true if we set the EVFILT_WRITE filter after calling connect because connect didn't synchornously complete. After the error has been delievered via read() a call to getsockopt(SO_ERROR) will return 0 indicating there is no error. This means we cannot rely upon the KQueue based kernel to deliver connection errors via the EVFILT_WRITE filter in the same way that the linux kernel does with the EPOLLOUT flag.
ce241bd introduced a change which depends upon the behavior of the EVFILT_WRITE being set and may prematurely stop writing to the OS as a result, becaues we assume the OS will notify us when the socket is writable. However the current work around for the above described behavior is to initialize the EVFILT_WRITE to true for connection oriented protocols. This leads to prematurely exiting from the flush() which may lead to deadlock.

Modifications:
- KQueue should check when an error is obtained from read() if the connectPromise has not yet been completed, and if not complete it with a ConnectException

Result:
No more deadlock in KQueue due to asynchronous connect workaround.
2018-02-20 11:01:49 -08:00
Norman Maurer
c6c0984b8e Ensure we always release the AddressEnvelope when doing DNS queries.
Motivation:

When we do DNS queries we need to ensure we always release the AddressEnvelope.

Modifications:

Also release the AddressEnvelope if the original resolution was done in the meantime and we did not cancel the extra query yet.

Result:

Should fix [#7713]
2018-02-20 12:06:21 +01:00
David Nault
f40ecc3f10 Fix Snappy decoding of large 2-byte literal lengths and copy offsets
Motivation:

The Snappy decoder was failing on valid inputs containing literals
with 2-byte lengths > 0x8000 or copies with 2-byte offsets >= 0x8000.

The decoder was also enforcing an artificially low offset limit of
0x7FFF, something the Snappy format description advises against,
and which prevents decoding valid inputs generated by other encoders.

Modifications:

Interpret 2-byte literal lengths and 2-byte copy offsets as unsigned
shorts, in accordance with the format description and reference
implementation.

Allow any positive offset value. Throw an appropriate exception
for negative values (which can theoretically occur due to arithmetic
overflow on 4-byte offsets, but are unlikely to occur in the wild).

Result:

The Snappy decoder can handle valid inputs that previously caused
it to throw exceptions.
2018-02-20 11:42:23 +01:00
Scott Mitchell
ce241bd11e Epoll flush/writabilityChange deadlock
Motivation:
b215794de3 recently introduced a change in behavior where writeSpinCount provided a limit for how many write operations were attempted per flush operation. However when the write quantum was meet the selector write flag was not cleared, and the channel unsafe flush0 method has an optimization which prematurely exits if the write flag is set. This may lead to no write progress being made under the following scenario:
- flush is called, but the socket can't accept all data, we set the write flag
- the selector wakes us up because the socket is writable, we write data and use the writeSpinCount quantum
- we then schedule a flush() on the EventLoop to execute later, however it the flush0 optimization prematurely exits because the write flag is still set

In this scenario the socket is still writable so the EventLoop may never notify us that the socket is writable, and therefore we may never attempt to flush data to the OS.

Modifications:
- When the writeSpinCount quantum is exceeded we should clear the selector write flag

Result:
Fixes https://github.com/netty/netty/issues/7729
2018-02-20 11:40:58 +01:00