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

HttpServerUpgradeHandler causing StringIndexOutOfBoundsException #11568

Closed
dpy1123 opened this issue Aug 11, 2021 · 2 comments · Fixed by #11569
Closed

HttpServerUpgradeHandler causing StringIndexOutOfBoundsException #11568

dpy1123 opened this issue Aug 11, 2021 · 2 comments · Fixed by #11569

Comments

@dpy1123
Copy link
Contributor

dpy1123 commented Aug 11, 2021

We are using netty as a proxy server. And added HttpServerUpgradeHandler incase client wants to talk in h2c.
But with a normal http1 request, we see the following exceptions.

	at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:98)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
	at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
	at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296)
	at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:714)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:650)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:576)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:493)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: java.lang.StringIndexOutOfBoundsException: String index out of range: -1
	at java.base/java.lang.AbstractStringBuilder.setLength(AbstractStringBuilder.java:275)
	at java.base/java.lang.StringBuilder.setLength(StringBuilder.java:85)
	at io.netty.handler.codec.http.HttpServerUpgradeHandler.upgrade(HttpServerUpgradeHandler.java:297)
	at io.netty.handler.codec.http.HttpServerUpgradeHandler.decode(HttpServerUpgradeHandler.java:239)
	at io.netty.handler.codec.http.HttpServerUpgradeHandler.decode(HttpServerUpgradeHandler.java:40)
	at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:88)
	... 22 more

As far as I can see, it seems like the if conditon should also add isEmpty().

            return false;
        }

@dpy1123
Copy link
Contributor Author

dpy1123 commented Aug 11, 2021

Work around can be fond in #11267
Implement your own shouldHandleUpgradeRequest, and check connection header there.

@chrisvest
Copy link
Contributor

@dpy1123 Yeah, a missing isEmpty() looks about right. Would you like to make a PR for this?

NiteshKant pushed a commit that referenced this issue Aug 12, 2021
…11569)

__Motivation__
Since request.headers().getAll() will never return null. And the check null condition will not work as expected.

__Modification__

Add isEmpty() checking as well.

__Result__

Fixes #11568
NiteshKant pushed a commit that referenced this issue Aug 12, 2021
…11569)

__Motivation__
Since request.headers().getAll() will never return null. And the check null condition will not work as expected.

__Modification__

Add isEmpty() checking as well.

__Result__

Fixes #11568
laosijikaichele pushed a commit to laosijikaichele/netty that referenced this issue Dec 16, 2021
…etty#11569)

__Motivation__
Since request.headers().getAll() will never return null. And the check null condition will not work as expected.

__Modification__

Add isEmpty() checking as well.

__Result__

Fixes netty#11568
laosijikaichele pushed a commit to laosijikaichele/netty that referenced this issue Dec 16, 2021
…etty#11569)

__Motivation__
Since request.headers().getAll() will never return null. And the check null condition will not work as expected.

__Modification__

Add isEmpty() checking as well.

__Result__

Fixes netty#11568
raidyue pushed a commit to raidyue/netty that referenced this issue Jul 8, 2022
…etty#11569)

__Motivation__
Since request.headers().getAll() will never return null. And the check null condition will not work as expected.

__Modification__

Add isEmpty() checking as well.

__Result__

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

Successfully merging a pull request may close this issue.

2 participants