bcgov / TheOrgBook

A public repository of verifiable claims about organizations. A key component of the Verifiable Organization Network.
http://von.pathfinder.gov.bc.ca
Apache License 2.0
78 stars 66 forks source link

Work-around for NPE on blank expiry date #678

Closed ianco closed 5 years ago

andrewwhitehead commented 5 years ago

@ianco What's being passed in exactly? A blank string or None shouldn't get that far.

Could have been a build from before this fix? https://github.com/bcgov/TheOrgBook/commit/b0cc47fe5deeaad76d3c2f824d2a9b427339e9d4

ianco commented 5 years ago

@cywolf I don't know what's getting passed through, but to reproduce setup an agent with a credential with an optional date field, and then submit with an empty date.

In the von-agent-template project, like this: https://github.com/bcgov/von-agent-template/blob/master/von-x-agent/config/schemas.yml#L34

Then on the UI: https://github.com/bcgov/von-agent-template/blob/master/von-x-agent/config/routes.yml#L49

... then fill in the form on the UI, leave the date blank, and submit.

This is recent (yesterday) and has all updates.

andrewwhitehead commented 5 years ago

@ianco It looks like the form is passing in a date, not a datetime, and parse_datetime is just returning None. So I guess the question is whether it should handle plain dates (and add a time) or throw an exception.

ianco commented 5 years ago

It's not an issue when I actually pass a value. The error occurs when I submit the form but leave the optional date value empty. If I submit the credential using the REST service and leave the attribute out completely it also works fine ...

andrewwhitehead commented 5 years ago

Can you check what it's actually submitting from the form? I don't think it's empty.

ianco commented 5 years ago

@cywolf what's the easiest way to do that? Everything is handled by back-end code (von-x, tob etc)

andrewwhitehead commented 5 years ago

Maybe the network inspector in the browser first, otherwise log the credential attributes in TOB. I can test it later but I don't have the agent set up at the moment.

ianco commented 5 years ago

Aha! expiry_date: "NaN"

andrewwhitehead commented 5 years ago

Looks like somebody added moment.js handling..

https://github.com/bcgov/von-agent-template/blob/a8ba71445fc5f0292a300ee504b055ec93e0ac32/von-x-agent/assets/js/main.js#L24

I added a fix to the OrgBook to reject the invalid value.

ianco commented 5 years ago

@cywolf what's the correction for the "moment handling" in main.js? This is from the code originally provided to the bc-reg agent (and is now in all of our agents)

ianco commented 5 years ago

Fixed the fix of the fix