eclipse-vertx / vert.x

Vert.x is a tool-kit for building reactive applications on the JVM
http://vertx.io
Other
14.32k stars 2.08k forks source link

TooLongFormFieldException With HTTP2 After Upgrade #5225

Closed winrid closed 5 months ago

winrid commented 5 months ago

I recently upgraded from Vertx 4.3.4 to 4.5.8. We started getting these errors in production for small (<2kb) messages:

io.netty.handler.codec.http.multipart.HttpPostRequestDecoder$TooLongFormFieldException
    at io.netty.handler.codec.http.multipart.HttpPostStandardRequestDecoder.offer(HttpPostStandardRequestDecoder.java:341)
    at io.netty.handler.codec.http.multipart.HttpPostStandardRequestDecoder.offer(HttpPostStandardRequestDecoder.java:53)
    at io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.offer(HttpPostRequestDecoder.java:278)
    at io.vertx.core.http.impl.Http1xServerRequest.onData(Http1xServerRequest.java:563)
    at io.vertx.core.http.impl.Http1xServerRequest.lambda$pendingHandler$0(Http1xServerRequest.java:350)
    at io.vertx.core.streams.impl.InboundBuffer.handleEvent(InboundBuffer.java:279)
    at io.vertx.core.streams.impl.InboundBuffer.write(InboundBuffer.java:157)
    at io.vertx.core.http.impl.Http1xServerRequest.handleContent(Http1xServerRequest.java:134)
    at io.vertx.core.impl.ContextImpl.execute(ContextImpl.java:313)
    at io.vertx.core.impl.DuplicatedContext.execute(DuplicatedContext.java:159)
    at io.vertx.core.http.impl.Http1xServerConnection.onContent(Http1xServerConnection.java:203)
    at io.vertx.core.http.impl.Http1xServerConnection.handleOther(Http1xServerConnection.java:189)
    at io.vertx.core.http.impl.Http1xServerConnection.handleMessage(Http1xServerConnection.java:176)
    at io.vertx.core.net.impl.ConnectionBase.read(ConnectionBase.java:159)
    at io.vertx.core.net.impl.VertxHandler.channelRead(VertxHandler.java:153)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
    at io.netty.channel.ChannelInboundHandlerAdapter.channelRead(ChannelInboundHandlerAdapter.java:93)
    at io.netty.handler.codec.http.websocketx.extensions.WebSocketServerExtensionHandler.channelRead(WebSocketServerExtensionHandler.java:87)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
    at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346)
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318)
...

Here are my HTTP opts:

    HttpServerOptions httpOpts = new HttpServerOptions();
    if (ssl && Config.getInstance().sslCertificatePath != null) {
      httpOpts.setPemKeyCertOptions(new PemKeyCertOptions()
        .setCertPath(Config.getInstance().sslCertificatePath)
        .setKeyPath(Config.getInstance().sslCertificateKeyPath));
      httpOpts.setSsl(true);
    }
    httpOpts.setTcpNoDelay(true);
    httpOpts.setTcpQuickAck(true);
    httpOpts.setSendBufferSize(1_000); // optimization: we don't send large messages. avg message size is 1kb
    httpOpts.setReceiveBufferSize(100); // optimization: we don't really receive messages.

The fix is to disable the max form buffered size, as we tried larger values and still ran into the exception:

httpOpts.setMaxFormBufferedBytes(-1);

It is pretty unclear what this option means. Is it the max value of a field? is it the max value of a buffer? Also it is not clear how the buffer size impacts parsing. I'd expect a smaller buffer to just impact performance, but somehow it is impacting validation. Thanks a bunch!

vietj commented 5 months ago

you should set setMaxFormFields(-1)

winrid commented 5 months ago

I'm sorry, I don't think that reply is really acceptable. Why is that? What is the change? What is the impact/purpose of the buffer validation? We still have the issue that the API is confusing, surprising, and not well documented.

Also, I tried to find a vertx changelog, and the tags don't have changes, and there is no "changelog.txt" file etc? I have to click through a blog post for each release to read the changes - is that correct?

winrid commented 5 months ago

Also, we're ignoring the fact that this seems like a bug. Why would I have to specify -1? Why would the default validation prevent handling small messages at all?

vietj commented 5 months ago

@winrid

winrid commented 5 months ago

Hello @vietj

Thanks for your reply and the links. I am trying to help create a reproducer. It seems to only happen in production, I think it may be related to using HTTPS. The service terminates ssl itself.

I don't do anything special. I am POSTING a JSON object with only a couple fields:

    HttpServerOptions httpOpts = new HttpServerOptions();
    if (ssl && Config.getInstance().sslCertificatePath != null) {
      httpOpts.setPemKeyCertOptions(new PemKeyCertOptions()
        .setCertPath(Config.getInstance().sslCertificatePath)
        .setKeyPath(Config.getInstance().sslCertificateKeyPath));
      httpOpts.setSsl(true);
    }
    httpOpts.setTcpNoDelay(true);
    httpOpts.setTcpQuickAck(true);
    httpOpts.setSendBufferSize(1_000); // we don't send large messages, just comments and presence updates. avg comment size is 1kb
    httpOpts.setReceiveBufferSize(100); // we don't really receive messages.

    Router router = Router.router(vertx);
    router.route().handler(BodyHandler.create());
    router.post("/send").handler((context) -> {
...

The client (nodejs example) can send something like:

axios.post('http://localhost:8000/send', JSON.stringify({
            "type": "new-comment",
            "broadcastId": "01a3a6ac-81bc-4fe6-936b-b16b8714ca5f",
            "timestamp": 1717529446743,
            "comment": {
                "_id": "gxC_OVaLX",
                "tenantId": "L177BUDVvSe",
                "urlId": "test",
                "urlIdRaw": "test",
                "url": "http://localhost:63342/abc/scripts/local-test-v2.html?_ijt=a7gc6ths0itu1ef4v17tg6oftp&_ij_reload=RELOAD_ON_SAVE",
                "pageTitle": "Local Test V2",
                "userId": "PKEzMbmJH3",
                "anonUserId": "anon:AMVzTAKwqf",
                "commenterEmail": "someone@somewhere.com",
                "commenterName": "someperson",
                "comment": "his is a longer comment! some stuff some stuff some stuff some stuff \nsome stuff some stuff some stuff some stuff some stuff some stuff some \nstuff some stuff some stuff some stuff some stuff some stuff some stuff \nsome stuff some stuff some stuff some stuff some stuff some stuff some \nstuff some stuff some stuff some stuff some stuff some stuff some stuff \nsome stuff some stuff some stuff some stuff some stuff some stuff some \nstuff some stuff some stuff some stuff some stuff some stuff some stuff \nsome stuff some stuff some stuff some stuff some stuff some stuff some \nstuff some stuff some stuff some stuff some stuff some stuff some stuff \nsome stuff some stuff some stuff some stuff some stuff some stuff some \nstuff some stuff some stuff some stuff some stuff some stuff some stuff \nsome stuff some stuff some stuff some stuff some stuff",
                "commentHTML": "his is a longer comment! some stuff some stuff some stuff some stuff<br />some stuff some stuff some stuff some stuff some stuff some stuff some<br />stuff some stuff some stuff some stuff some stuff some stuff some stuff<br />some stuff some stuff some stuff some stuff some stuff some stuff some<br />stuff some stuff some stuff some stuff some stuff some stuff some stuff<br />some stuff some stuff some stuff some stuff some stuff some stuff some<br />stuff some stuff some stuff some stuff some stuff some stuff some stuff<br />some stuff some stuff some stuff some stuff some stuff some stuff some<br />stuff some stuff some stuff some stuff some stuff some stuff some stuff<br />some stuff some stuff some stuff some stuff some stuff some stuff some<br />stuff some stuff some stuff some stuff some stuff some stuff some stuff<br />some stuff some stuff some stuff some stuff some stuff",
                "date": "2024-06-04T19:30:45.853Z",
                "localDateString": "Tue Jun 04 2024 12:30:45 GMT-0700 (Pacific Daylight Time)",
                "localDateHours": 12,
                "votes": 0,
                "votesUp": 0,
                "votesDown": 0,
                "expireAt": null,
                "verified": false,
                "notificationSentForParent": false,
                "notificationSentForParentTenant": false,
                "reviewed": false,
                "avatarSrc": null,
                "isSpam": false,
                "hasImages": false,
                "hasLinks": false,
                "hasCode": false,
                "approved": true,
                "locale": "en_us",
                "isByAdmin": false,
                "isByModerator": false,
                "fromProductId": 0,
                "badges": [],
                "domain": "localhost",
                "__v": 0,
                "hashTags": []
            }
        }));