Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Backend crash when opening hostile-named submission detail #1157

Closed
issa-tseng opened this issue Jun 19, 2024 · 8 comments · Fixed by #1158
Closed

Backend crash when opening hostile-named submission detail #1157

issa-tseng opened this issue Jun 19, 2024 · 8 comments · Fixed by #1158
Assignees
@alxndrsn

Comments

@issa-tseng
Copy link
Contributor

issa-tseng commented Jun 19, 2024 •
edited
Loading

  1. Go to here: https://staging.getodk.cloud/#/projects/1/forms/'%3D%2B%2F*-451%25%2F%25/submissions
  2. Open the submission detail page of any of the strangely named submissions
  3. Server crash, user sees a 404
Problem [Error]: Could not find the resource you were looking for.
    at Object.result [as notFound] (lib/util/problem.js:45:31)
    at filterFields (lib/formats/odata.js:610:48)
    at lib/formats/odata.js:660:94
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
@lognaturel
Copy link
Member

It didn't ring a bell when you mentioned it but now I'm pretty sure this was @lindsay-stevens pushing the limits with pyodk. I think we discussed with @matthew-white and decided that we didn't need to support non-uuid instance ids. If that sounds right to everyone, I think we can close this!

@lognaturel
Copy link
Member

Well, could we surface the error and not do anything instead of crashing? Maybe that's not any easier than just supporting that case?

@alxndrsn
Copy link
Contributor

we didn't need to support non-uuid instance ids

Should submissions with non-uuid instanceId values be rejected when initially submitted?

@issa-tseng
Copy link
Contributor Author

i'm not sure how the user input was inserted, but it feels like the hard crash should be addressed at least. bad input shouldn't crash the server, that's a DOS.

@alxndrsn
Copy link
Contributor

alxndrsn commented Jun 19, 2024 •
edited
Loading

@issa-tseng where are you seeing the server crashing? I'm seeing the 404, but having trouble recreating the crash.

Recreated by running 20 requests in parallel:

{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}{"message":"Could not find the resource you were looking for.","code":404.1}<html>
<head><title>502 Bad Gateway</title></head>
<body>
<center><h1>502 Bad Gateway</h1></center>
<hr><center>nginx</center>
</body>
</html>
<html>
<head><title>502 Bad Gateway</title></head>
<body>
<center><h1>502 Bad Gateway</h1></center>
<hr><center>nginx</center>
</body>
</html>
<html>
<head><title>502 Bad Gateway</title></head>
<body>
<center><h1>502 Bad Gateway</h1></center>
<hr><center>nginx</center>
</body>
</html>

alxndrsn pushed a commit to alxndrsn/odk-central-backend that referenced this issue Jun 19, 2024
resources/odata: don't crash with surprising instanceId
30a0a75 
Closes getodk#1157
alxndrsn pushed a commit to alxndrsn/odk-central-backend that referenced this issue Jun 19, 2024
resources/odata: don't crash with surprising instanceId
a3f6d0c 
Closes getodk#1157
alxndrsn pushed a commit to alxndrsn/odk-central-backend that referenced this issue Jun 19, 2024
resources/odata: don't crash with surprising instanceId
768bfe3 
Closes getodk#1157
alxndrsn pushed a commit to alxndrsn/odk-central-backend that referenced this issue Jun 19, 2024
resources/odata: don't crash with surprising instanceId
3f6d4ac 
Closes getodk#1157
@alxndrsn
Copy link
Contributor

It looks like the crash is caused by an unhandled error thrown by the synchronous call to selectFields() on line 52:

central-backend/lib/resources/odata.js

Lines 49 to 55 in 7afef66

const singleRecord = endpoint.odata.json(({ Forms, Submissions, env }, { auth, params, query, originalUrl }) =>
getForm(Forms, auth, params)
.then((form) => Promise.all([
Forms.getFields(form.def.id).then(selectFields(query, getTableFromOriginalUrl(originalUrl))),
Submissions.getForExport(form.id, getUuid(params.uuid), draft).then(getOrNotFound)
])
.then(([fields, row]) => singleRowToOData(fields, row, env.domain, originalUrl, query))));

@alxndrsn alxndrsn mentioned this issue Jun 19, 2024
Merged
@issa-tseng
Copy link
Contributor Author

the crash happens without any parallel reqs for me. my guess is you need it because docker is restarting the server faster than you can see. i click the link once my server crashes 100% of the time.

@lognaturel
Copy link
Member

I want to be clear that I agree the crash should be prevented so glad to see #1158! I was also initially being fooled by the Docker restart.

@github-project-automation github-project-automation bot moved this to 🕒 backlog in ODK Central Oct 15, 2024
@matthew-white matthew-white moved this from 🕒 backlog to ✏️ in progress in ODK Central Oct 15, 2024
@github-project-automation github-project-automation bot moved this from ✏️ in progress to ✅ done in ODK Central Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alxndrsn alxndrsn

Labels
None yet
Projects
ODK Central
Status: ✅ done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

resources/odata: don't crash with surprising instanceId alxndrsn/odk-central-backend
3 participants
@issa-tseng @alxndrsn @lognaturel