Allow non destructive reads of request body #426
Conversation
| String remoteUri = routingTargetHandler.getRoutingDestination(multiReadHttpServletRequest); | ||
| proxyRequestHandler.postRequest(body, multiReadHttpServletRequest, asyncResponse, URI.create(remoteUri)); |
There was a problem hiding this comment.
Why would this apply only to POST and not to others GET, DELETE?
Would it not be safer to use MultiReadHttpServletRequest instead of HttpServletRequest in all cases?
including RoutingGroup codes?
There was a problem hiding this comment.
the GETs and DELETEs won't have bodies, so getReader and getInputStream will be null either way
There was a problem hiding this comment.
True, hmm.. how about using one type just for unifcation for all cases? even if it doesn't have body?
There was a problem hiding this comment.
Yeah, I think that is what @oneonestar was suggesting with the plan to replace HttpServeletRequest with a dedicated class like TrinoRequestContext. This is the way to go, but would broaden the scope of this PR by quite a bit. So for now I think we can get by with HttpServletRequest, using the MultiRead implementation when there is a body
| String remoteUri = routingTargetHandler.getRoutingDestination(multiReadHttpServletRequest); | ||
| proxyRequestHandler.postRequest(body, multiReadHttpServletRequest, asyncResponse, URI.create(remoteUri)); |
There was a problem hiding this comment.
True, hmm.. how about using one type just for unifcation for all cases? even if it doesn't have body?
|
Description
HttpServeletRequestonly supports destructive reads of a request's body. This change ensures different components can access the request body without interfering with one another.Fixes #427
Fixes #428
Additional context and related issues
The body of a request is consumed by the
ServeletHandlers that supply thebodyargument toRouteToBackendResource.postHandler. It is necessary to re-associate the body with the request so it can be used by downstream query id extraction logic and routing rules.Release notes
(x ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: