search

gorilla / rpc

Package gorilla/rpc is a golang foundation for RPC over HTTP services.
https://gorilla.github.io
BSD 3-Clause "New" or "Revised" License
595 stars 179 forks source link

Make request body readable and changeable in interceptFunc and beforeFunc #81

Closed groovili closed 8 months ago

groovili commented 4 years ago

Hi!

This PR gives users the ability to access and change request.Body in functions registered with RegisterInterceptFunc and RegisterBeforeFunc. I've already described it in issue #80. Also, I've added couple of simple tests to check ability to change request data in these functions.

Changes:

  1. In rpc/v2 close request.Body after the execution of beforeFunc's and interceptFunc's. Update codec request info after calls to functions above.
  2. Read request body bytes, decode it to codec format and provide bytes.Buffer and request.Body for underlying functions in v2/json, v2/json2 and v2/protorpc codecs.

Of course, exists a better way to do that, but it will require changes in the signature of RegisterInterceptFunc and RegisterBeforeFunc and it would be breaking changes. If both of these methods will have an original *http.Request as input parameter, they could be executed before the creation of codec. In this case, users can access and alter request data, and only after that it would be read by the codec and marshaled to service request params. But since these changes are breaking, it's not an option at the moment, probably it can fit the next version or release.

Would be nice to know your opinion, thanks!

AlexVulaj commented 8 months ago

LGTM as well @jaitaiwan . @groovili thanks for the contribution! Do you mind resolving your branch conflicts so we can go ahead and merge this? Thanks!

groovili commented 8 months ago

@AlexVulaj @jaitaiwan thanks for the review! Merged with latest changes and updated the CL to not use deprecated ioutil. Please take another look to proceed with merging.

codecov[bot] commented 8 months ago

Codecov Report

Attention: Patch coverage is 64.15094% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 71.70%. Comparing base (39123e3) to head (422b3f1).

Files Patch % Lines
v2/json2/server.go 47.05% 8 Missing and 1 partial :warning:
v2/server.go 63.63% 3 Missing and 1 partial :warning:
v2/json/server.go 78.57% 2 Missing and 1 partial :warning:
v2/protorpc/server.go 72.72% 2 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #81 +/- ## ========================================== + Coverage 71.44% 71.70% +0.25% ========================================== Files 15 15 Lines 844 887 +43 ========================================== + Hits 603 636 +33 - Misses 181 189 +8 - Partials 60 62 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.