springernature / boomcatch

A standalone, node.js-based beacon receiver for boomerang.
GNU General Public License v3.0
110 stars 33 forks source link

Ampersands in restiming URLs #83

Open JonHouse opened 7 years ago

JonHouse commented 7 years ago

Hi, I hope this isn't a rather stupid thing to bring up, but I've been battling with an issue where the site I'm working on would request a url with ampersands in breaking the JSON parsing in boomcatch.

So requesting something like: https://use.typekit.net/af/4713c5/000000000000000000017690/27/l?primer=09b6306563171828e867a80b0e487a34c56b49b8efde4541b63ffd44535e9a56&fvd=n4&v=3

would make boomerang send a timing object to boomcatch like this (looks good): "use.typekit.net/af/4713c5/000000000000000000017690/27/l?primer=09b6306563171828e867a80b0e487a34c56b49b8efde4541b63ffd44535e9a56&fvd=n4&v=3":"3p,2g,28,13,13,b,a,a,2111fw,8720"

However on the boomcatch sode of things it breaks. By the time it gets to the mapper it has been mangled and lookes like: {"use.typekit.net/af/4713c5/000000000000000000017690/27/l?primer=09b6306563171828e867a80b0e487a34c56b49b8efde4541b63ffd44535e9a56', fvd: 'n4', v: [ '3":"3p" .... etc

After a bit of digging I discovered this was occurring in index.js between lines 693 and 699

   state.body = decodeURIComponent(state.body);

    if (request.headers['content-type'] === 'text/plain') {
        return JSON.parse(state.body);
    }

    return qs.parse(state.body, { parameterLimit: Infinity });

If decoded data is sent to qs.parse it breaks out the ampersand of a nested querystring and completely breaks the JSON at the point where the ampersand would occur in the restiming data. The only way I got it to work is if I moved the decodeURIComponent function into the if statement:

    if (request.headers['content-type'] === 'text/plain') {
        state.body = decodeURIComponent(state.body);
        return JSON.parse(state.body);
    }

    return qs.parse(state.body, { parameterLimit: Infinity });

I can't say if this is the ideal solution, it could break other parts of the application but it seems to work for me at the moment.

Why is the whole body being treated as if its a query string and being sent through qs.parse?

Is there a better way to solve this?

Thanks.

itsMattShull commented 6 years ago

Can confirm that this worked for me as well. Creating a pull request to include this and a fix for https://github.com/springernature/boomcatch/issues/49