From 8319c73e394862042d198d771035a15101497d79 Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Wed, 31 Jan 2024 20:36:20 +0530 Subject: [PATCH 1/7] Handle client level header size validation response before sending to server --- .../http2_configuration_request_limit_config_test.bal | 2 +- .../contractimpl/common/states/Http2StateUtil.java | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal index 83924c0dd3..6cba28c384 100644 --- a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal +++ b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal @@ -147,7 +147,7 @@ function testHttp2ValidHeaderLength() returns error? { //Tests the behaviour when header size is greater than the configured threshold // TODO: Enable after fixing this issue : https://github.com/ballerina-platform/ballerina-standard-library/issues/3963 @test:Config { - enable: false + enable: true } function testHttp2InvalidHeaderLength() returns error? { http:Client limitClient = check new ("http://localhost:" + http2RequestLimitsTestPort3.toString(), diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/common/states/Http2StateUtil.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/common/states/Http2StateUtil.java index e8ed1daf54..e72fd6ae8a 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/common/states/Http2StateUtil.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/common/states/Http2StateUtil.java @@ -43,6 +43,7 @@ import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelPromise; import io.netty.handler.codec.http.DefaultLastHttpContent; import io.netty.handler.codec.http.HttpContent; import io.netty.handler.codec.http.HttpHeaderNames; @@ -320,9 +321,13 @@ public static void writeHttp2Headers(ChannelHandlerContext ctx, OutboundMsgHolde return; } } - - encoder.writeHeaders(ctx, streamId, http2Headers, dependencyId, weight, false, 0, endStream, - ctx.newPromise()); + ChannelPromise promise = ctx.newPromise(); + encoder.writeHeaders(ctx, streamId, http2Headers, dependencyId, weight, false, 0, endStream, promise); + promise.addListener((ChannelFutureListener) channelFuture -> { + if (!channelFuture.isSuccess()) { + outboundMsgHolder.getResponseFuture().notifyHttpListener(channelFuture.cause()); + } + }); encoder.flowController().writePendingBytes(); ctx.flush(); From fd570727628e5d8ebf0203b0c34e632db2297027 Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Wed, 31 Jan 2024 20:55:51 +0530 Subject: [PATCH 2/7] Remove test config --- .../http2_configuration_request_limit_config_test.bal | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal index 6cba28c384..7121cbc310 100644 --- a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal +++ b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal @@ -145,10 +145,7 @@ function testHttp2ValidHeaderLength() returns error? { } //Tests the behaviour when header size is greater than the configured threshold -// TODO: Enable after fixing this issue : https://github.com/ballerina-platform/ballerina-standard-library/issues/3963 -@test:Config { - enable: true -} +@test:Config {} function testHttp2InvalidHeaderLength() returns error? { http:Client limitClient = check new ("http://localhost:" + http2RequestLimitsTestPort3.toString(), http2Settings = {http2PriorKnowledge: true}); @@ -159,9 +156,7 @@ function testHttp2InvalidHeaderLength() returns error? { // Tests the fallback behaviour when header size is greater than the configured http2 service // TODO: Enable after fixing this issue : https://github.com/ballerina-platform/ballerina-standard-library/issues/3963 -@test:Config { - enable: false -} +@test:Config {} function testHttp2Http2ServiceInvalidHeaderLength() returns error? { http:Client limitClient = check new ("http://localhost:" + requestLimitsTestPort5.toString(), http2Settings = {http2PriorKnowledge: true}); From 6f96fe9cc6ca66080d4f46e277b56e1f290990b2 Mon Sep 17 00:00:00 2001 From: Dilan Sachintha Nayanajith Date: Wed, 31 Jan 2024 22:51:18 +0530 Subject: [PATCH 3/7] Update pull-request.yml --- .github/workflows/pull-request.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 0fc46e6b60..f633c496d2 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -6,5 +6,5 @@ jobs: call_workflow: name: Run PR Build Workflow if: ${{ github.repository_owner == 'ballerina-platform' }} - uses: ballerina-platform/ballerina-standard-library/.github/workflows/pull-request-build-template.yml@main + uses: ballerina-platform/ballerina-standard-library/.github/workflows/pull-request-build-template.yml@dilanSachi-patch-1 secrets: inherit From 0a5b890a9dceaec9e4616b39786614f581b114d6 Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Thu, 1 Feb 2024 09:09:29 +0530 Subject: [PATCH 4/7] Update test case --- ...configuration_request_limit_config_test.bal | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal index 7121cbc310..92fe98ebed 100644 --- a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal +++ b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal @@ -149,9 +149,14 @@ function testHttp2ValidHeaderLength() returns error? { function testHttp2InvalidHeaderLength() returns error? { http:Client limitClient = check new ("http://localhost:" + http2RequestLimitsTestPort3.toString(), http2Settings = {http2PriorKnowledge: true}); - http:Response response = check limitClient->get("/lowRequestHeaderLimit/invalidHeaderSize", {"X-Test": getLargeHeader()}); + http:Response|http:Error response = limitClient->get("/lowRequestHeaderLimit/invalidHeaderSize", {"X-Test": getLargeHeader()}); //431 Request Header Fields Too Large - test:assertEquals(response.statusCode, 431, msg = "Found unexpected output"); + if response is http:Error { + test:assertTrue(response is http:ClientError); + test:assertEquals(response.message(), "Header size exceeded max allowed size (600)"); + } else { + test:assertEquals(response.statusCode, 431, msg = "Found unexpected output"); + } } // Tests the fallback behaviour when header size is greater than the configured http2 service @@ -160,8 +165,13 @@ function testHttp2InvalidHeaderLength() returns error? { function testHttp2Http2ServiceInvalidHeaderLength() returns error? { http:Client limitClient = check new ("http://localhost:" + requestLimitsTestPort5.toString(), http2Settings = {http2PriorKnowledge: true}); - http:Response response = check limitClient->get("/http2service/invalidHeaderSize", {"X-Test": getLargeHeader()}); - test:assertEquals(response.statusCode, 431, msg = "Found unexpected output"); + http:Response|http:Error response = limitClient->get("/http2service/invalidHeaderSize", {"X-Test": getLargeHeader()}); + if response is http:Error { + test:assertTrue(response is http:ClientError); + test:assertEquals(response.message(), "Header size exceeded max allowed size (850)"); + } else { + test:assertEquals(response.statusCode, 431, msg = "Found unexpected output"); + } } //Tests the behaviour when payload size is greater than the configured threshold From 7a6aaf7553b4589546dd08b254965493dd5ffc81 Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Thu, 1 Feb 2024 11:22:08 +0530 Subject: [PATCH 5/7] Update changelog.md --- changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.md b/changelog.md index 3dc64a4314..757f5fdea4 100644 --- a/changelog.md +++ b/changelog.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Fixed - [Remove unused import from Http2StateUtil](https://github.com/ballerina-platform/ballerina-library/issues/5966) +- [Fix client getting hanged when multiple requests are sent which exceed `maxHeaderSize`](https://github.com/ballerina-platform/ballerina-library/issues/6000) ## [2.10.5] - 2023-12-06 From b65f4fbc39363631a6a5a978571e6b71b159a414 Mon Sep 17 00:00:00 2001 From: Dilan Sachintha Nayanajith Date: Thu, 1 Feb 2024 12:31:27 +0530 Subject: [PATCH 6/7] Update pull-request.yml --- .github/workflows/pull-request.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index f633c496d2..f0cd3fd7fc 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -6,5 +6,5 @@ jobs: call_workflow: name: Run PR Build Workflow if: ${{ github.repository_owner == 'ballerina-platform' }} - uses: ballerina-platform/ballerina-standard-library/.github/workflows/pull-request-build-template.yml@dilanSachi-patch-1 + uses: ballerina-platform/ballerina-standard-library/.github/workflows/pull-request-build-template.yml@workflow-test secrets: inherit From 5b5e7f33faa39bd550c166be51a2c746d53883b6 Mon Sep 17 00:00:00 2001 From: Dilan Sachintha Nayanajith Date: Thu, 1 Feb 2024 14:39:13 +0530 Subject: [PATCH 7/7] Update pull-request.yml --- .github/workflows/pull-request.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index f0cd3fd7fc..0fc46e6b60 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -6,5 +6,5 @@ jobs: call_workflow: name: Run PR Build Workflow if: ${{ github.repository_owner == 'ballerina-platform' }} - uses: ballerina-platform/ballerina-standard-library/.github/workflows/pull-request-build-template.yml@workflow-test + uses: ballerina-platform/ballerina-standard-library/.github/workflows/pull-request-build-template.yml@main secrets: inherit