Skip to content

Commit

Permalink
Improve virtual request parsing (#7287)
Browse files Browse the repository at this point in the history
* Improve virtual request parsing

Relevant for socket.io usage, and usage from tests.  (`sails.request()`)

* Update req.js

---------

Co-authored-by: Eric <[email protected]>
  • Loading branch information
mikermcneil and eashaw authored Jul 21, 2023
1 parent 406a120 commit 4a023dc
Showing 1 changed file with 17 additions and 12 deletions.
29 changes: 17 additions & 12 deletions lib/router/req.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,27 @@ module.exports = function buildRequest (_req) {
}
else {

// TODO: send a PR to mock-req with a fix for this
if (_req.headers && typeof _req.headers === 'object') {
// Strip undefined headers
_.each(_req.headers, function (headerVal, headerKey) {
if (_.isUndefined(headerVal)){
for (let headerKey of Object.keys(_req.headers)) {
// Strip undefined headers
if (undefined === _req.headers[headerKey]) {
delete _req.headers[headerKey];
}
});
// Make sure all remaining headers are strings
_req.headers = _.mapValues(_req.headers, function (headerVal /*, headerKey*/) {
if (typeof headerVal !== 'string') {
headerVal = ''+headerVal+'';
// Make sure all remaining headers are strings
if (typeof _req.headers[headerKey] !== 'string') {
try {
_req.headers[headerKey] = ''+_req.headers[headerKey];
// FUTURE: This behavior is likely being relied upon by apps, so we can't just change it.
// But in retrospect, it would probably be better to straight-up reject this here if it's not
// a string, since HTTP header values are always supposed to be strings; or at least primitives.
// So maybe reject non-primitives, reject `null`, and then accept primitives, but be smart about
// this, especially in the context of what the client is doing.
} catch (unusedErr) {
delete _req.headers[headerKey];
}
}
return headerVal;
});
}
}//∞
}//fi

// Create a mock IncomingMessage stream.
req = new MockReq({
Expand Down

2 comments on commit 4a023dc

@Lovinity
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit breaks websocket sessions / req.session .

See #7297

@mikermcneil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much @Lovinity. Resolution developing here: #7297 (comment)

Please sign in to comment.