-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Data leak between requests #6944
Comments
@bobthebuilderio Thanks for posting! We'll take a look as soon as possible. In the mean time, there are a few ways you can help speed things along:
Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly. For help with questions about Sails, click here. |
As an additional piece of information, it is worth noting that we do use DynamoDb Waterline Adapter. We added some extra feature support ourselves internally as it was a bit behind. Yet I still don't see how the cases above could occur regardless of anything else. There seem to be a Major / fundamental issue somewhere. |
I do not think the issue has to do with the ORM itself, as the data if logged before the array handling function described in issue 2 is correct, but get's mixed after the function gets called. |
Can you share any actual code or create a minimal github repo that can reproduce these issues? |
This is the main controller that fetches the data - THIS IS THE CODE THAT IS GENERATING THE ISSUE. I will follow up with another comment of the file where issue was fixed. `module.exports = { friendlyName: sails.('controllers.orderItem.arango.listByOrderId.friendlyName'), inputs: { exits: { fn: async function (inputs, exits) {
}, /** */ const itemsListToReturn = []; for (item of itemsList) {
} return itemsListToReturn; /** */ order.pickupImageBags = !_.isArray(pickupImageBags) return order; /** */ const itemsList2 = _.groupBy(itemsList, 'orderAlphaId'); if (Object.keys(itemsList2).length > 1) { } ` |
THIS IS THE CODE THAT FIXED THE ISSUE. AS YOU CAN SEE SIMPLE CHANGE TO THE CODE. NOTHING THAT IS OBVIOUSLY WRONG OR COULD CAUSE AN ISSUE LIKE THIS. SOMETHING HAS TO DO WITH ASYNC/AWAIT / LOOPS. As the issue arises when we have promise.all or awaits in for loops. Might be just talking gibberish :) ! `module.exports = { friendlyName: sails.('controllers.orderItem.arango.listByOrderId.friendlyName'), inputs: { exits: { fn: async function (inputs, exits) {
}, /** */ for (let i = 0; i < itemsList.length; i++) {
} } /** */ order.pickupImageBags = !_.isArray(pickupImageBags) return order; |
@johnabrams7 I think this should not be labeled as a Question, rather should be something more serious and needs immediate attention. What do you think ? |
@bobthebuilderio Yes, good point - went ahead and labeled it Bug instead 👍 Huge thanks for all the effort and dedication with this, along with a workaround! 🎉 To confirm, which specific DynamoDb Waterline Adapter are you using with Sails? There appears to be multiple. |
@johnabrams7 sorry for the delay, was overwhelmed. I will ask someone from the team to provide a clean repo that could reproduce the issue. Hopefully will be able to demostrate it as this appears on a production environment, so we might have to create a script that simulate simultaneous hits. Will update you soon. |
@bobthebuilderio Now worries, that sounds great! We'll look forward to checking it out. |
Node version: node:8.12-alpine
Sails version (sails): "^1.2.3"
ORM hook version (sails-hook-orm): "^2.1.1"
Sockets hook version (sails-hook-sockets): "^1.4.0"
Unfortunately we faced two major issues on 2 important api built with sailsjs.
ISSUE 1: Somehow response data from one request say to '/user/login' end up being swapped with someone's else request response. So users on our apps got swapped and saw the other's info,.
ISSUE 2: Similarly, two parallel requests to an api say 'list-items' end up mixing items from both users together in the resonse. So one user gets some of his items and some of the other user's items & same with the other user.
What we did to solve the problem, was to refactor the code a bit:
1- We stopped using things like promise.all() and did some refactoring which solved ISSUE 1
2- We had a function receives items list as input goes into a for loop ( for x of y ) to call two awaits db calls for each of the items, create some additional properly for the item, push into a new array and return this array. This array was getting mixed up.
Regardless of our code and whether it could be written in a better way or not, how on earth is data being swapped like this ? is there anyone who knows or faced something like this with saisjs before ?
Thank you !
The text was updated successfully, but these errors were encountered: