-
Notifications
You must be signed in to change notification settings - Fork 57
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
Nuxt security with Socket.io and xssValidator make POST requests hanging #472
Comments
Hey, Thanks for reporting this issue. XSS validator works for both Get and Post requests so probably it is cataching the request and cannot parse it properly. Do you maybe have access to the logs of your app to see if it says something in the console? |
There is no relevant log regarding this issue. I already digged in your module and tried to do some early returns in the Can you reproduce with my link ? I tried to provide a reproduction as minimal as possible, with the alternatives where it is working |
Now that I think about it, I think I noticed the error occurred around here when debugging: (Notice the // OK
if (rules.xssValidator.methods && rules.xssValidator.methods.includes(event.node.req.method) && false) { // ...
const valueToFilter = event.node.req.method === "GET"
? getQuery(event)
: event.node.req.headers["content-type"]?.includes("multipart/form-data")
? await readMultipartFormData(event)
: await readBody(event);
// Escaping here makes the POST request hanging, maybe is it related to the above line ?
if (valueToFilter && Object.keys(valueToFilter).length && false) { |
Thanks for the additional digging! Yes, it could be related to that. Would you be interested in creating a Pull Request with a fix for that? I think we would need to support a new case of sockets that wasn't handled before as we were working with pure http requests with body, then we added the multipart form data and today we would also need to support a new case of sockets :) I could provide all the help needed 🚀 |
Just tried it through stackblitz, can't seem to reproduce it: I made sure to use the module as is, without disabling xss. Was only able to reproduce it under one condition which appears to be completely unrelated, will investigate some more. Update: turns out it was an edge case that was indeed related to it, will test the fix locally before making a PR @noook do you allow me to use your repro as a test case? |
Of course! My repository exists for this very exact reason |
@noook @Baroshem a little update on this: const promise = ((event.node.req as any)[RawBodySymbol] = new Promise<Buffer>(
(resolve, reject) => {
const bodyData: any[] = [];
event.node.req
.on('error', (err) => {
reject(err);
})
.on('data', (chunk) => {
bodyData.push(chunk);
})
.on('end', () => {
resolve(Buffer.concat(bodyData));
});
}
)) |
Mentioning the code with this permalink: Are you suggesting a fix would be to store the result of the promise in a variable attached to the event, so that it can be read again later on ? I could open an issue for that and eventually provide the fix. |
Well, the issue is not really there, it's in the second promise with bodyData array. I'm experimenting with it to figure out how this could work. |
Great @GalacticHypernova that you have found it. I think you could talk with Pooya about it :) Should we close this issue as it is an upstream? |
I think we should, though it's definitely interesting why websockets (or at least this specific websocket) can't use data to read. Ideally it shouldn't do it. I'm not even sure it is an h3 issue. Theoretically it could be an issue of the websocket library itself, I'll try a different websocket and see |
Closing the issue. If there is a need, please reopen or create a new one :) |
So sorry, got caught up with real life and forgot about this. I think it's definitely not something that needs fixing here though because that would be a workaround rather than an issue. Last I checked I didn't receive a response on the issue I opened in h3 about this, but hopefully when it gets fixed it resolves it for socket as well |
Version
nuxt-security:
2.0.0-beta.5
nuxt:
3.11.2
Reproduction Link
https://github.com/noook/nuxt-security-socket-io-repro
Steps to reproduce
What is Expected?
Successful connection to socket.io server and successful requests.
xssValidator
option, it works.GET
requests it works as well.What is actually happening?
POST requests never end.
Notes
I don't know if those options are required for the project to work
The text was updated successfully, but these errors were encountered: