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.

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

isFalse() and non-string query parameters #806

Closed
matthew-white opened this issue Mar 23, 2023 · 3 comments · Fixed by #1099
Closed

isFalse() and non-string query parameters #806

matthew-white opened this issue Mar 23, 2023 · 3 comments · Fixed by #1099
Assignees

Comments

@matthew-white
Copy link
Member

isFalse() throws an error if a query parameter is not blank or string:

TypeError: x.toLowerCase is not a function
  File "/usr/odk/lib/util/http.js", line 22, col 43, in isFalse
    const isFalse = (x) => (!isBlank(x) && (x.toLowerCase() === 'false'));
  File "/usr/odk/lib/util/db.js", line 391, col 9, in Function.fromSubmissionCsvRequest
    if (isFalse(query.groupPaths))
  File "/usr/odk/lib/resources/submissions.js", line 270, col 42, in <anonymous>
    const options = QueryOptions.fromSubmissionCsvRequest(query);
  File "/usr/odk/lib/util/http.js", in runMicrotasks
  File "node:internal/process/task_queues", line 96, col 5, in processTicksAndRejections

For example, we've seen this error for the (invalid) query parameter deletedFields=%5B%22false%22%2C%22false%22%5D. isFalse() seems to work fine for valid user input, but it'd be nice if it were more resilient to user error. For example, Express allows a query parameter to be an array, but isFalse() doesn't account for that possibility.

isTrue() should probably be updated in a similar way as isFalse().

@JosephVoid
Copy link
Contributor

Hi @matthew-white,

I'd like to work on this. But I couldn't understand what the purpose of the isTrue and isFalse functions are.
Plus, just on top of my head, wouldn't typeof x === string be a solution to this.

Thanks

@matthew-white
Copy link
Member Author

Hi @JosephVoid! isTrue() and isFalse() just check whether the value of a particular query parameter is true or false. I guess the main thing the functions do is checking that in a case-insensitive way: the values TRUE and FALSE are also allowed. I think you're probably right that typeof x is all that's needed!

@JosephVoid
Copy link
Contributor

Hi @matthew-white ,
I've created a PR, let me know what you think.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ done
Development

Successfully merging a pull request may close this issue.

2 participants