Bug 29322 - Date validation can be sketchy on the API
Summary: Date validation can be sketchy on the API
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-26 13:41 UTC by Martin Renvoize
Modified: 2021-11-18 11:08 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2021-10-26 13:41:54 UTC
Jonathan found some interesting errors with the date-time validation when writing extra tests for bug 24850.

Passing:
    "due_date": "2021-10-27 23:59",
Results in:
    => "Does not match date-time"

But passing:
    "due_date": "2021-10-27 23:59:00",
Results in:
    => "Missing property

Shouldn't they both result in:
    => "Does not match date-time"
Comment 1 Jonathan Druart 2021-11-18 10:11:14 UTC
The current situation on master is:

PUT http://kohadev-intra.mydnsname.org:8081/api/v1/holds/1
    {
        "pickup_library_id": "FPL",
        "priority": 1,
        "suspended_until": "2021-10-27 23:59"
    }

=> 
  "message": "Does not match date-time format.",
  "path": "/body/suspended_until"

PUT http://kohadev-intra.mydnsname.org:8081/api/v1/holds/1
    {
        "pickup_library_id": "FPL",
        "priority": 1,
        "suspended_until": "2021-10-27 23:59:00"
    }

=>     "error": "Something went wrong, check Koha logs for details."

With, in logs:
[2021/11/18 10:10:47] [ERROR] PUT /api/v1/holds/1: unhandled exception (Mojo::Exception)<<The given date (2021-10-27 23:59:00) does not match the date format (rfc3339) at /kohadevbox/koha/Koha/DateUtils.pm line 193.>> Koha::REST::Plugin::Exceptions::__ANON__ /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73)
Comment 2 Jonathan Druart 2021-11-18 10:12:58 UTC
Note that:

        "suspended_until": null
=> 
"message": "Expected string - got null.",


        "suspended_until": ""
=>
"message": "Does not match date-time format.",

Shouldn't we allow null here? (not directly related however).
Comment 3 Tomás Cohen Arazi 2021-11-18 10:28:47 UTC
(In reply to Jonathan Druart from comment #1)
> The current situation on master is:
> 
> PUT http://kohadev-intra.mydnsname.org:8081/api/v1/holds/1
>     {
>         "pickup_library_id": "FPL",
>         "priority": 1,
>         "suspended_until": "2021-10-27 23:59"
>     }
> 
> => 
>   "message": "Does not match date-time format.",
>   "path": "/body/suspended_until"
> 
> PUT http://kohadev-intra.mydnsname.org:8081/api/v1/holds/1
>     {
>         "pickup_library_id": "FPL",
>         "priority": 1,
>         "suspended_until": "2021-10-27 23:59:00"
>     }
> 
> =>     "error": "Something went wrong, check Koha logs for details."
> 
> With, in logs:
> [2021/11/18 10:10:47] [ERROR] PUT /api/v1/holds/1: unhandled exception
> (Mojo::Exception)<<The given date (2021-10-27 23:59:00) does not match the
> date format (rfc3339) at /kohadevbox/koha/Koha/DateUtils.pm line 193.>>
> Koha::REST::Plugin::Exceptions::__ANON__
> /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73)

Any 'Something went wrong' message means an uncaught exception. Once detected, we should catch it correctly and return the right message.
Comment 4 Jonathan Druart 2021-11-18 10:47:19 UTC
Cannot we teach the validator what is the date format we expect? It's weird that it catches some but not all bad formatted dates.
Comment 5 Tomás Cohen Arazi 2021-11-18 11:08:20 UTC
(In reply to Jonathan Druart from comment #4)
> Cannot we teach the validator what is the date format we expect? It's weird
> that it catches some but not all bad formatted dates.

The validator is supposed to expect RFC3339 valid dates. Maybe it is time to try what happens in newer versions.

In the meantime, we could use a regex instead of telling it is a date-time. I would (in that case) document on the spec that those regex expect rfc3339 date/date-time to avoid confusion.