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

Retrieve the raw request body #2255

Open
1 of 2 tasks
thomaslule opened this issue Nov 25, 2021 · 6 comments
Open
1 of 2 tasks

Retrieve the raw request body #2255

thomaslule opened this issue Nov 25, 2021 · 6 comments
Labels

Comments

@thomaslule
Copy link

Please avoid duplicates

Context

I use nock to test that my server correctly sends webhook notifications. The requests are signed using the header x-hub-signature, a header containing a sha256 hash of the binary body data.

Ideally I would like to retrieve the raw body data in order to check that the header is consistent with this data.

Alternatives

As an alternative I can:

  • retrieve the body object value
  • retrieve the header value
  • use JSON.stringify() and Buffer.from() to recreate the (hopefully) same binary data
let notificationBody;
let signatureHeader;
nock("https://www.example.com", {
  reqheaders: {
    "x-hub-signature": value => {
      signatureHeader = value;
      return true;
    }
  }
})
  .post("/callback", body => {
    notificationBody = body;
    return true;
  })
  .reply(200);
// and once I sent the request:
const expectedSignature = `sha256=${createHmac("sha256", secret)
  .update(Buffer.from(JSON.stringify(notificationBody)))
  .digest("hex")}`;
expect(signatureHeader).toEqual(expectedSignature);

But with this approach, I am abusing the reqheaders property and the body function to extract the info, then a lot can go wrong (for example if the JSON isnt formatted the same way in my tested code or if the encoding isnt the same).

I feel that there could be a better way to do that.

If the feature request is accepted, would you be willing to submit a PR?

  • yes
@gr2m gr2m changed the title Retrieve the request body as Buffer Retrieve the raw request body Nov 26, 2021
@gr2m
Copy link
Member

gr2m commented Nov 26, 2021

I thought there was a way to access the raw request body but cannot recall it right now. Can you check the tests if there is one doing that? That's what I'd do as a first step

@thomaslule
Copy link
Author

Thanks for your reply!

I looked into the tests, I found that we can match requests using raw buffer body: https://github.com/nock/nock/blob/main/tests/test_body_match.js#L239

But in this case, where you dont know beforehand what the body will be, this doesnt help

@gr2m
Copy link
Member

gr2m commented Nov 29, 2021

where you dont know beforehand what the body will be

Just curious: how come you don't know the body? I'd expect a test to run in a fully controlled environment, where the same input always results in the same output?


Off-topic question: can you elaborate on what you are trying to build? It seems that you are attempting to emulate webhook requests sent by GitHub? Are you trying to implement custom webhook requests for a GitHub App? If so, I think I'd rather implement custom routes

@mastermatt
Copy link
Member

Off-hand, I think you access the req directly using this in the post callback.
The base issue IMO is one we've discussed a lot in the past in that there is no way to access requests after the fact for clean assertions, which means tests are forced to pull data out in the middle of matching.

@thomaslule
Copy link
Author

Just curious: how come you don't know the body? I'd expect a test to run in a fully controlled environment, where the same input always results in the same output?

The request body is generated by my tested code, I know most things about it except some details: it will contain a random uuid that I dont know beforehand (but I could probably make it predictable with a mock) + the formatting of the json is considered implementation detail. Technically I know how it's formatted but I would prefer that my tests dont need to know that.

Off-topic question: can you elaborate on what you are trying to build? It seems that you are attempting to emulate webhook requests sent by GitHub? Are you trying to implement custom webhook requests for a GitHub App? If so, I think I'd rather implement custom routes

Of course! I work on a system that will emit webhook requests, it is unrelated to github but it mostly works the same way.

Off-hand, I think you access the req directly using this in the post callback.

Oh yes, apparently you can do that, but the object doesnt contain the body of the request.

The base issue IMO is one we've discussed a lot in the past in that there is no way to access requests after the fact for clean assertions, which means tests are forced to pull data out in the middle of matching.

Yeah I agree. I understand that the primary goal of this lib is to define predictable responses to http calls (and it does that very well 👏 ), but sometimes we also want to make assertions on the requests 😅

@gr2m
Copy link
Member

gr2m commented Nov 30, 2021

I am refactoring the intercept logic right now, you can follow my progress at #2247, the current code is at https://github.com/nock/nock/tree/code-review/modules/intercept-node-http. I had to expose an extra method which does not exist on Node's native http.ClientRequest called nockGetRequestBodyAsBuffer in order to retrieve the request body for matching purposes.

I'm not a big fan of defining a dedicated method but I actually couldn't figure out how to read the request body for a native http.ClientRequest object. My guess is the current workaround in nock is there because of the same reason?

The base issue IMO is one we've discussed a lot in the past in that there is no way to access requests after the fact for clean assertions, which means tests are forced to pull data out in the middle of matching.

Once I have the first version ready of the @nock/intercept-node-http module it would be good to discuss how to use this low level module for clean request body assertions. Then we can adopt that for a future nock version

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

No branches or pull requests

3 participants