terrymanu commented on PR #38745:
URL: https://github.com/apache/shardingsphere/pull/38745#issuecomment-4562001583
## Review Result
Merge Verdict: Mergeable
I re-reviewed the latest local HEAD `fecc9a9cda2` for the MCP session
attribution changes. I did not find remaining blocking issues.
### What Was Verified
- Existing-session validation no longer creates a first attribution binding
during later requests. If an existing session receives attribution headers but
has no bound attribution, the validator now rejects the request with `400`.
- Session attribution is still bound on the initialization POST path after
`Mcp-Session-Id` is produced.
- Attribute header keys are normalized consistently for case-insensitive
HTTP header matching.
- HTTP header name validation now follows token-style field-name validation
instead of only rejecting whitespace and colon.
- Regression tests cover the matched, mismatched, and unbound attribution
paths, mixed-case attribute headers, initialization binding, and invalid
attribution header configuration.
### Verification
- `./mvnw -pl mcp/api,mcp/bootstrap,mcp/core,mcp/support -am -DskipITs
-Dspotless.skip=true
-Dtest=SessionAttributionResolverTest,ShardingSphereServerTransportSecurityValidatorTest,YamlHttpTransportConfigurationSwapperTest,StreamableHttpMCPServletTest,MCPSessionManagerTest,MCPRequestScopeTest,MCPClientSafetyPolicyTest,MCPModelFirstContractPayloadBuilderTest
-Dsurefire.failIfNoSpecifiedTests=false test`
- Result: Passed
- `./mvnw checkstyle:check -Pcheck -T1C`
- Result: Passed
LGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
