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

Fix broken client tls negotiation #11999

Merged

Conversation

lorban
Copy link
Contributor

@lorban lorban commented Jul 3, 2024

When the Jetty client needs to negotiate TLS with the server, it fails to do so and cannot open a connection.
The only indication is the following debug log:

2024-07-03 10:40:19.579:DEBUG:oeji.FillInterest:HttpClient@59d016c9-24: onFail FillInterest@40dda3e8{null}
java.nio.ReadOnlyBufferException
	at java.base/java.nio.HeapByteBufferR.put(HeapByteBufferR.java:246)
	at org.eclipse.jetty.util.BufferUtil.put(BufferUtil.java:460)
	at org.eclipse.jetty.util.BufferUtil.append(BufferUtil.java:549)
	at org.eclipse.jetty.io.ssl.SslConnection$SslEndPoint.fill(SslConnection.java:830)
	at org.eclipse.jetty.io.NegotiatingClientConnection.fill(NegotiatingClientConnection.java:102)
	at org.eclipse.jetty.io.NegotiatingClientConnection.onFillable(NegotiatingClientConnection.java:84)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:99)
	at org.eclipse.jetty.io.ssl.SslConnection$SslEndPoint.onFillable(SslConnection.java:574)
	at org.eclipse.jetty.io.ssl.SslConnection.onFillable(SslConnection.java:390)
	at org.eclipse.jetty.io.ssl.SslConnection$2.succeeded(SslConnection.java:150)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:99)
	at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:478)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:441)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:293)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.produce(AdaptiveExecutionStrategy.java:195)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:979)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1209)

This happens because we mistakenly pass a read-only empty byte buffer to a fill() method that tries to append to it, so it fails with an exception instead of returning that zero byte was copied.

Fixes #11965

…eption in SslEndPoint.fill()

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
… fill() is called with an empty buffer

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban lorban added the Bug For general bugs on Jetty side label Jul 3, 2024
@lorban lorban requested review from gregw and joakime July 3, 2024 08:49
@lorban lorban self-assigned this Jul 3, 2024
Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's going to be difficult to know when to use one constant vs the newly introduced one.

Can we make the old constant non-read only?

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban lorban requested a review from sbordet July 8, 2024 10:20
@lorban lorban requested a review from sbordet July 9, 2024 08:14
Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment.

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban lorban requested a review from sbordet July 9, 2024 10:41
@lorban lorban merged commit 8f5207e into jetty-12.0.x Jul 10, 2024
10 checks passed
@lorban lorban deleted the fix/jetty-12.0.x/11965-broken-client-tls-negotiation branch July 10, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Client: Some HTTP/2 requests are never sent
2 participants