ehn-dcc-development / eu-dcc-schema

Schema for the ehn DCC payload
Apache License 2.0
164 stars 59 forks source link

Date of bird validation pattern #99

Closed Raschmann closed 3 years ago

Raschmann commented 3 years ago

The regullar expression for validation date is wrong:

[19|20][0-9][0-9]-(0[1-9]|1[0-2])-([0-2][1-9]|3[0|1])

The last past allows 01-09, 11-19, 21-29, 30, 31 bat not allows 10 and 20

'2010-07-10' does not match '[19|20][0-9][0-9]-(0[1-9]|1[0-2])-([0-2][1-9]|3[0|1])'

Failed validating 'pattern' in schema['properties']['dob']: {'description': 'Date of Birth of the person addressed in the DGC. ISO ' '8601 date format restricted to range 1900-2099', 'examples': ['1979-04-14'], 'format': 'date', 'pattern': '[19|20][0-9][0-9]-(0[1-9]|1[0-2])-([0-2][1-9]|3[0|1])', 'title': 'Date of birth', 'type': 'string'}

On instance['dob']: '2010-07-10'

gabywh commented 3 years ago

On which version is this?

kbobrowski commented 3 years ago

This regex also allows for non-existing dates, like 2021-02-31

vaubaehn commented 3 years ago

This regex also allows for non-existing dates, like 2021-02-31

Same here: https://github.com/ehn-dcc-development/ehn-dcc-schema/blob/a603410d760fefc9073931c8c807759d9714c136/DCC.combined-schema.json#L52

vaubaehn commented 3 years ago

@Raschmann @kbobrowski Please check whether your questions are related to RCs or final versions: https://github.com/ehn-dcc-development/ehn-dcc-schema/issues/101#issuecomment-860192978

gabywh commented 3 years ago

This regex also allows for non-existing dates, like 2021-02-31

Same here: https://github.com/ehn-dcc-development/ehn-dcc-schema/blob/a603410d760fefc9073931c8c807759d9714c136/DCC.combined-schema.json#L52

Thanks for pointing out the obvious ;) Perhaps there's a reason the regex is like it is in the DCC schema: https://github.com/ehn-dcc-development/ehn-dcc-schema/wiki/FAQ#what-do-the-typical-processing-stages-look-like-for

Raschmann commented 3 years ago

After updating to 1.3.0 version there was no new occurrence of failed validation.

gabywh commented 3 years ago

From recent discussions with implementers, I think people are slowly starting to get why the DCC Schema is spec'ed as it is - there certainly appears to be a change of opinion now with how to implement verifier apps which comes much closer to what I described a while back here: https://github.com/ehn-dcc-development/ehn-dcc-schema/wiki/FAQ#what-do-the-typical-processing-stages-look-like-for

ioggstream commented 3 years ago

It seems that the iso8601 format mentioned here https://github.com/ehn-dcc-development/ehn-dcc-schema/wiki/FAQ#why-not-use-seconds-since-epoch-instead-of-the-iso-8601-format-for--date-and-date-time is too wide wrt the actual supported format. Some improvements were made in ISO 8601-1:2019.

It could be possible to limit the spec to RFC5424 https://datatracker.ietf.org/doc/html/rfc5424#section-6.2.3 with UTC, though json-schema date-time format references RFC3339.

ISO8601 supports a broad range of formats and requires parsers that are timezone aware unless UTC is enforced: even in these cases there's the need to address leap seconds. This is the reason why in cryptographic context, "UNIX Timestamp" can be preferred. In this case the "epoch" is well defined in https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xbd_chap03.html#tag_21_03_00_20.

gabywh commented 3 years ago
  1. too wide - your (incorrect) assumption here is that DCC JSON Schema validation is the final arbiter. I won't repeat all the already-repeated discussions on this topic. 2.UNIX timestamp - thanks for the ref, but was already aware of it (e.g. I was working on the millenium bug for the City of London Stock Exchange way back at the 1999 / 2000 change, so yeah, I guess I was already aware of it more than a couple of decades ago already). UNIX timestamp is defined for UNIX, as the name implies. Won't re-open the discussion here (1) as per my above point: your assumption on the role of purely JSON level validation is incorrect (2) topic already documented in the FAQ as it already arose several times in the past (which is why it is in the FAQ), and (3) eHN spec is ISO 8601 (IMHO: correctly so), so it's staying.