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

Wrong Content-Length header when event has Content-Length header and its payload contains whitespace #5

Closed
niutski opened this issue Feb 14, 2019 · 5 comments

Comments

@niutski
Copy link

niutski commented Feb 14, 2019

I'm trying to use Smee.io to use webhooks from Bitbucket Cloud to our Jenkins behind a firewall. Bitbucket sends webhook events with content-length header, but the body it sends contains whitespace. smee.io server correctly parses the incoming JSON, but then it implicitly removes whitespace but does not change the content-length header when it forwards the event to the client. This causes Jenkins to fail in parsing the request body because the Content-Length header does not match the actual length of the content.

To test:

  1. Create a fake endpoint for forwarded events, such as:
/** testserver.js */
const express = require('express');
const http = require('http');
const bodyParser = require('body-parser');
const app = express();

app.post('/', bodyParser.json(), (req, res) => {
    console.log("Received request");
    res.sendStatus(200);
});

const server = http.Server(app);
const port = 3000;
server.listen(port, () => {
    console.log('Listening on port', port);
});
  1. Run the fake endpoint with node testserver.js
  2. Run smee-client: smee.js --port 3000 (copy the channel ID from the output)
  3. curl -X POST --data '{ "ok": true }' -H "Content-Length: 14" -H "Content-Type: application/json" https://smee.io/[YOUR_CHANNEL_HERE]
  4. ERROR: The fake server never manages to parse the json because the content length doesn't match
  5. If you try curl -X POST --data '{"ok":true}' -H "Content-Length: 11" -H "Content-Type: application/json" https://smee.io/[YOUR_CHANNEL_HERE] things work as expected
@niutski
Copy link
Author

niutski commented Feb 15, 2019

I have a few options for solving this:

  1. Recalculate the Content-Length header in the client before forwarding the event
  2. Recalculate the Content-Length header in the server
  3. Omit the Content-Length header when the client forwards events
  4. Omit the Content-Length header when the server sends the event to the client

Any thoughts or preferences? I can make a PR for any of the options that you feel is the best way to solve the problem.

Here's also a test to show the issue:

it('message payload length matches content-length header', async (done) => {
  const payload = '{ "payload": true }'
  await request(server).post(channel)
    .set('content-type', 'application/json')
    .set('content-length', payload.length)
    .send(payload)

  events.addEventListener('message', (msg) => {
    const data = JSON.parse(msg.data)
    const msgPayload = msg.data.match(/"body":(\{.*?\})/)[1]
    const contentLength = Number(data['content-length'])
    expect(msgPayload.length).toEqual(contentLength)

    done()
  })
})

Running it gives the output:

Expected value to equal:
  19
Received:
  16

@JasonEtco JasonEtco transferred this issue from probot/smee-client Apr 2, 2019
@niutski
Copy link
Author

niutski commented Aug 12, 2019

This is also one way to fix the issue: probot/smee-client#122

@benzman81
Copy link

I had the same problem. Using the test from @niutski I could verify that adding 'content-length': JSON.stringify(req.body).length, right after query: req.query, in server.js fixes the issue.

@benzman81
Copy link

I have to correct the solution of my last comment. You should not use the string length like this JSON.stringify(req.body).length but this Buffer.byteLength(JSON.stringify(req.body))

@gr2m
Copy link
Contributor

gr2m commented Jun 16, 2021

would you like to send a pull request to fix it?

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

No branches or pull requests

3 participants