-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Read latest request body instead of snapshot copy #6
Conversation
@bhargavmodi or @danny-gallagher would you have any feedback on this PR please? |
@@ -3,7 +3,7 @@ package oauth | |||
import ( | |||
"crypto/rsa" | |||
"errors" | |||
"io/ioutil" | |||
"io" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although the changes looks good, the library build fails to import the io module. this version 1.12 still assumes io/util is not deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we will merge those changes directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will incorporate these changes as part of this PR: #8
Addressed in: #8 |
Addressing issue #7
When reading the request body to be signed, the actual body could have been modified since when it was first created (for example when using an encryption middleware).
Instead of using
req.GetBody()
(which returns a snapshot copy, see https://github.com/golang/go/blob/master/src/net/http/request.go#L919), we should read the actual latest content of the body.In addition, I have swapped the deprecated
ioutil
package forio
PR checklist
master
branch