Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Channel promise of a write is successful, although SSL handshake fails. #7378

Closed
sauroter opened this issue Nov 7, 2017 · 1 comment
Closed
Assignees
Labels
Milestone

Comments

@sauroter
Copy link
Contributor

sauroter commented Nov 7, 2017

Expected behavior

When a SSL handshake fails, a channel promise of a write operation should fail aswell.

Actual behavior

The channel promise succeeds.

Steps to reproduce

$ git clone https://github.com/s-gheldd/netty-reproducer.git
$ cd netty-reproducer
$ mvn test

Minimal yet complete reproducer code (or URL to code)

https://github.com/s-gheldd/netty-reproducer

Netty version

4.1.16.Final

(last version that had the expected behavior 4.1.13.Final)

JVM version (e.g. java -version)

java version "1.7.0_80"
Java(TM) SE Runtime Environment (build 1.7.0_80-b15)
Java HotSpot(TM) 64-Bit Server VM (build 24.80-b11, mixed mode)

OS version (e.g. uname -a)

Darwin ghelds-MacBook-Pro.local 15.6.0 Darwin Kernel Version 15.6.0: Mon Oct  2 22:20:08 PDT 2017; root:xnu-3248.71.4~1/RELEASE_X86_64 x86_64
Scottmitch added a commit to Scottmitch/netty that referenced this issue Nov 7, 2017
complete

Motivation:
SslHandler removes a Buffer/Promise pair from
AbstractCoalescingBufferQueue when wrapping data. However it is possible
the SSLEngine will not consume the entire buffer. In this case
SslHandler adds the Buffer back to the queue, but doesn't add the
Promise back to the queue. This may result in the promise completing
immediately in finishFlush, and generally not correlating to the
completion of writing the corresponding Buffer

Modifications:
- AbstractCoalescingBufferQueue#addFirst should also support adding the
ChannelPromise
- In the event of a handshake timeout we should immediately fail pending
writes immediately to get a more accurate exception

Result:
Fixes netty#7378.
@Scottmitch
Copy link
Member

See #7380

@Scottmitch Scottmitch self-assigned this Nov 7, 2017
@Scottmitch Scottmitch added this to the 4.1.17.Final milestone Nov 7, 2017
andsel pushed a commit to andsel/netty that referenced this issue Nov 15, 2017
complete

Motivation:
SslHandler removes a Buffer/Promise pair from
AbstractCoalescingBufferQueue when wrapping data. However it is possible
the SSLEngine will not consume the entire buffer. In this case
SslHandler adds the Buffer back to the queue, but doesn't add the
Promise back to the queue. This may result in the promise completing
immediately in finishFlush, and generally not correlating to the
completion of writing the corresponding Buffer

Modifications:
- AbstractCoalescingBufferQueue#addFirst should also support adding the
ChannelPromise
- In the event of a handshake timeout we should immediately fail pending
writes immediately to get a more accurate exception

Result:
Fixes netty#7378.
kiril-me pushed a commit to kiril-me/netty that referenced this issue Feb 28, 2018
complete

Motivation:
SslHandler removes a Buffer/Promise pair from
AbstractCoalescingBufferQueue when wrapping data. However it is possible
the SSLEngine will not consume the entire buffer. In this case
SslHandler adds the Buffer back to the queue, but doesn't add the
Promise back to the queue. This may result in the promise completing
immediately in finishFlush, and generally not correlating to the
completion of writing the corresponding Buffer

Modifications:
- AbstractCoalescingBufferQueue#addFirst should also support adding the
ChannelPromise
- In the event of a handshake timeout we should immediately fail pending
writes immediately to get a more accurate exception

Result:
Fixes netty#7378.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants