Skip to content

Don't store request specific data into the Layer object#5426

Closed
Congelli501 wants to merge 1 commit intoexpressjs:masterfrom
blgCloud:master
Closed

Don't store request specific data into the Layer object#5426
Congelli501 wants to merge 1 commit intoexpressjs:masterfrom
blgCloud:master

Conversation

@Congelli501
Copy link
Copy Markdown

@Congelli501 Congelli501 commented Feb 1, 2024

Currently, the path & params of the latest request is stored into the last matched layer.
The "Layer" object should be immutable: a request should not modify its attributes.

This means that parameters are kept into memory after the end a request.

For example, this code will show the latest value of the parameter, as the value is stored until a new request is made:

const express = require('./index.js');
const app = express();
const port = 3000;

app.get('/secret/:token', (req, res) => {
	res.send('Hello World!');
});

app.listen(port, () => {
	console.log(`Example app listening on port ${port}`);
});

setInterval(() => {
	console.log(app._router.stack.at(2).params);
}, 1000);
curl localhost:3000/secret/super_secret

Result:

Example app listening on port 3000
undefined
undefined
undefined
{ token: 'super_secret' }
{ token: 'super_secret' }

@dougwilson
Copy link
Copy Markdown
Contributor

Hello, and thank you for your pull request. Apologies, as I would have mentioned it before if there was an issue, but pull requests against the router files should be made over at https://github.com/pillarjs/router which is synced here (and removed outright in 5.x). If you do reopen, please be sure to include tests cases, as we need those to accept a PR.

@dougwilson dougwilson closed this Feb 2, 2024
@dougwilson dougwilson changed the title [Security] Don't store request specific data into the Layer object Don't store request specific data into the Layer object Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants