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

Search endpoints should validate dates individually #66

Open
atschabu opened this issue Jan 17, 2024 · 4 comments
Open

Search endpoints should validate dates individually #66

atschabu opened this issue Jan 17, 2024 · 4 comments
Labels

Comments

@atschabu
Copy link
Contributor

Looking at location and session service implementations, we currently only validate the from/to dates if both are set. Specification allows for none, either or both to be defined. Instead of checking only if both are defined, we should validate them individually

if (dateFrom != null && dateTo != null) validateDates("dateFrom", dateFrom, "dateTo", dateTo)

@lilgallon
Copy link
Member

You are absolutely right, this is an issue

@lilgallon
Copy link
Member

lilgallon commented Jan 18, 2024

Actually, what should we validate if only dateFrom is set or dateTo is set? validateDates currently only checks if to is after from

@atschabu
Copy link
Contributor Author

yeah ... that's my bad for not debuging this more deeply first ... I didn't realize that we are already using Instant at that point, so the actual date validation is done by Server when parsing. The issue is, that those throw DateTimeParseException which leads to a HTTP 500, instead of a client side error.

                        dateFrom = dateFrom?.let { Instant.parse(it) },
                        dateTo = dateTo?.let { Instant.parse(it) },

Simply adding another catch on the server side wouldn't help, as we wouldn't be able to distinguish between client and server error, in case this happens somewhere else. Similar issue with the .toInt() calls.

I'd suggest we add a few helpers for extracting specific types from params, so that we can produce proper response ... eg

fun HttpRequest.queryParamAsInstant(key: String): Instant?
fun HttpRequest.queryParamAsInt(key: String): Int?

they could convert numberformat and date format exceptions, and turn them into validation exceptions

@lilgallon
Copy link
Member

You are right, your suggestion is great, we should implement it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants