search

smart-on-fhir / client-py

Python SMART on FHIR client
http://docs.smarthealthit.org
Other
591 stars 211 forks source link

FHIRDateTime does not check expected format of input string properly #168

Closed jeremy-viyamd closed 3 months ago

jeremy-viyamd commented 3 months ago

Overview

FHIRDateTime does not check expected format of input string properly

Example

the string "2024-07-23T12:27:16.321661" matches the full regex in the fhir spec for dateTime

however, the following code

from fhirclient.models.fhirdatetime import FHIRDateTime
FHIRDateTime("2024-07-23T12:27:16.321661")

reports the following error

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jeremy.ong/viyamd/repo-env/lib/python3.11/site-packages/fhirclient/models/fhirdatetime.py", line 45, in __init__
    super().__init__(jsonval)
  File "/Users/jeremy.ong/viyamd/repo-env/lib/python3.11/site-packages/fhirclient/models/fhirdate.py", line 44, in __init__
    raise ValueError("does not match expected format")
ValueError: does not match expected format

Root Cause

the regex in the fhir spec for dateTime is not equivalent to the regex used to check the format, which was introduced in #166

p2-apple commented 3 months ago

Note that the example "2024-07-23T12:27:16.321661" above does not include a time zone, which must always be given if a time is present, so this particular case should indeed fail.

jeremy-viyamd commented 3 months ago

my understanding is that we're adhering to the fhir spec regex for dateTime, which is also suggested by this comment

the fhir spec regex for dateTime is ([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)(-(0[1-9]|1[0-2])(-(0[1-9]|[1-2][0-9]|3[0-1])(T([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\.[0-9]{1,9})?)?)?(Z|(\+|-)((0[0-9]|1[0-3]):[0-5][0-9]|14:00)?)?)?

i checked my example string using regex101 and it matches the full regex: https://regex101.com/r/bLeTE5/1

so even though the time zone is not included, this particular case should succeed because it matches the full fhir spec regex for dateTime

jeremy-viyamd commented 3 months ago

nevermind, the fhir spec says "If hours and minutes are specified, a timezone offset SHALL be populated."

weird, because the provided regex allows timezone offset to be omitted

p2-apple commented 3 months ago

You're right! That's an issue in R5 and newer, I've filed it here: https://jira.hl7.org/browse/FHIR-46453