Skip to content
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

Adding of RSA-SHA256 signature and oauth_body_hash parameter #40

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

rvflash
Copy link

@rvflash rvflash commented May 14, 2019

Hi,

First at all, thanks for your work on this package.

The purpose of this pull request is to add the following features:

  1. Add the management of the signature method RSA-SHA256.
  2. Add the management of the oauth_body_hash parameter in OAuth header. In order to not break the method's signature, a new one is created to manage it: SetAuthorizationHeaderWithBody.
    1- Fixes some golint errors.
  3. Using of constants instead of plain text for header and parameters.
  4. Using of context package instead of golang.org/x/net/context to remove external dependency. Go version required >1.7.

Thanks,

@garyburd
Copy link
Member

Strip the PR back to the features listed in the title and golang.org/x/net/context > context go1.7 conversion. Because my time is limited, I don't want to review or even discuss the cosmetic changes.

Given that the caller must keep a copy of the body on hand for the request, does it make sense to specify the body with a []byte instead of an io.Reader? If an io.Reader is used because the body might be a large file on disk, then SignatureMethod.hash should io.Copy the body to the hash.

@rvflash
Copy link
Author

rvflash commented May 28, 2019

Sorry for the late return, I also have limited time. As you want, I just wanted to help by decreasing the number of lint errors. Regarding the io.Reader, I will not argue but, rather than taking in strings or []byte slices, in this kind of usage, I use an io.Reader for data sources (work with every type).

@rvflash
Copy link
Author

rvflash commented Aug 2, 2019

Hi Gary, gentle reminder :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants