requestFilter won't filter out body field

[nati-mask]

Say I want to log the request body most of the times (so I'll add it to the requestWhitelist) But I also use the requestFilter callback option, and there I'll conditionally return undefined for the body prop (there ARE sensitive bodies!) We can clearly see in those lines: https://github.com/bithavoc/express-winston/blob/32a7747dfd32213b8a316b9c2b09becf8f8c7c29/index.js#L338-L344 That the filteredBody will get filled with my unwanted request body props (passing the same requestFilter to the filterObject in order to filter out body props is pointless, because it was intended to filter request props, not body props) And then, here at those lines: https://github.com/bithavoc/express-winston/blob/32a7747dfd32213b8a316b9c2b09becf8f8c7c29/index.js#L349-L355 This filteredBody will unconditionally bring back the body to my log-request! That's not only contradicts the docs:

requestFilter: function (req, propName) { return req[propName]; }, // A function to filter/return request values, defaults to returning all values allowed by whitelist. If the function returns undefined, the key/value will not be included in the meta.

...There is no exception mentioned for the body request value! That's also not aligned with the behavior of the responseFilter (which will filter-out the response body) But also, in my opinion, poses a security risk, because people will are using filters to filter out sensitive data. A fast solution could be just add a condition like that:

if (filteredRequest && filteredRequest.body) {
    if (filteredBody) {
        filteredRequest.body = filteredBody;
    } else {
        delete filteredRequest.body;
    }
}

But I don't sure this "patch" is a good idea, because as said, why pass this requestFilter to filter body props at all...

[bithavoc]

Seems like a bug, but if we fix it I think is also a breaking change. We're planning a 5.x series, we could fix the inconsistency there. Thoughts?

[boaz-amit]

@bithavoc Thanks for the quick response. To avoid breaking changes, how about adding an explicit boolean option like requestExcludeBody?

It's not the most consistent or elegant solution, but it would allow to painlessly avoid the issue by just changing line 334 from:

if (req.body !== undefined) {
//...
}

to

if (req.body !== undefined && !options.requestExcludeBody) {
//...
}

[bithavoc]

That would work, mind including some tests reproducing it and fixing it?

[nati-mask]

PR is here: #258