feat(server): support configurable max request payload size#646
feat(server): support configurable max request payload size#646jskjw157 wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
jskjw157
force-pushed
the
feature/configurable-max-payload-size
branch
2 times, most recently
from
1a92a5e to
ddf9bc9
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
@kpavlov Hey! I’ve addressed the feedback on this PR and all checks are passing now. Would you mind taking a look when you get a chance? Thanks! |
|
|
||
| configureTransportEndpoint(transport) | ||
|
|
||
| val oversizedPayload = "x".repeat(customMaxSize.toInt() + 1) |
There was a problem hiding this comment.
nit: Maybe let's use small valid payload and set the max size to payload length + 1. It ensures that rejection reason is size.
There was a problem hiding this comment.
Done — combined with the next nit into a parameterized test using a small 64-byte payload, with maxSize set relative to payload length (length-1, length, length+1).
| } | ||
|
|
||
| @Test | ||
| fun `POST at exactly custom max request body size is not rejected as payload too large`() = testApplication { |
There was a problem hiding this comment.
nit: This and previous tests could be combined into one parametrized test
There was a problem hiding this comment.
Done — merged into a single @ParameterizedTest with @MethodSource covering three boundary cases.
| return null | ||
| } | ||
|
|
||
| val body = call.receiveText() |
There was a problem hiding this comment.
nit: If there is a content length header and it's bigger than max body size, then receiving text is a waste of time and memory. Request can be immediately rejected.
There was a problem hiding this comment.
Already covered — Content-Length is checked before receiveText() (line 676). The body-length check after receiveText() is a fallback for missing or inaccurate Content-Length headers.
| val transport = StreamableHttpServerTransport( | ||
| StreamableHttpServerTransport.Configuration( | ||
| enableJsonResponse = true, | ||
| maxRequestBodySize = customMaxSize, |
There was a problem hiding this comment.
- test for negative value throwong
IllegalArgumentException
There was a problem hiding this comment.
Done — added assertFailsWith for Configuration(maxRequestBodySize = -1).
jskjw157
force-pushed
the
feature/configurable-max-payload-size
branch
from
ddf9bc9 to
43a79ff
Compare
|
lgtm |
kpavlov
force-pushed
the
feature/configurable-max-payload-size
branch
from
43a79ff to
3777c6d
Compare
kpavlov
force-pushed
the
feature/configurable-max-payload-size
branch
from
3777c6d to
3559b94
Compare
kpavlov
force-pushed
the
feature/configurable-max-payload-size
branch
from
3559b94 to
84952a6
Compare
3 participants
Summary
Add
maxRequestBodySizeparameter toStreamableHttpServerTransport.Configuration, allowing consumers to set a custom request body size limit lower than the default 4 MB.maxRequestBodySize: Longproperty toConfigurationwith a default of 4 MB (4,194,304bytes)MAXIMUM_MESSAGE_SIZEconstant with the configurable value inparseBody()Longfor content-length comparison to avoidIntoverflow on large valuesMotivation and Context
Closes #521
The
StreamableHttpServerTransporthas a hardcoded 4 MB request body limit. Consumers who need a lower limit currently have to manually count the request payload size before passing it to the SDK. This change makes the limit configurable viaConfiguration.How Has This Been Tested?
POST with custom max request body size rejects oversized payload— verifiesPayloadTooLargeis returned for payloads exceeding the custom limitPOST at exactly custom max request body size is not rejected as payload too large— verifies payloads at exactly the limit are not rejected by the size check./gradlew :kotlin-sdk-server:jvmTest --tests "*.StreamableHttpServerTransportTest"Breaking Changes
None. The new parameter has a default value matching the previous hardcoded limit.
Types of changes
Checklist