[DICOM archive] Ensure that settings necessary for anonymization exist #4165
Conversation
johnsaigle
added
Category: Bug
davidblader
removed
Release: Add to release notes
| @@ -106,6 +167,9 @@ class Dicom_Archive extends \NDB_Menu_Filter | |||
| */ | |||
| function toJSON() | |||
| { | |||
| /* Ensure that the server is well configured to prevent displaying | |||
| * data that is not properly anonmyized. | |||
| */ | |||
There was a problem hiding this comment.
This comment seems to be out of place
There was a problem hiding this comment.
This comment is still here
johnsaigle
added
the
State: Needs work
| 'LegoPhantomRegex', | ||
| 'LivingPhantomRegex', | ||
| 'patientIDRegex', | ||
| ); |
There was a problem hiding this comment.
I don't see any reference to these in this file. I only see them referenced from the view session page, so I don't think this is the right place for this.
Unless I'm missing something, I think this should either:
- Be moved to the viewsession page, where the configs are used and it just needs to add a !empty check or
- Be checked by the Module class for the module, not the menu page's class.
There was a problem hiding this comment.
Theyr'e used by dicomarchiveanonymizer.class.inc (I mentioned this in my description but maybe I wasn't clear). Would you prefer that the checks go there instead?
Alternatively I can make the comment above more clear about where those settings are used.
There was a problem hiding this comment.
They should checked close to the thing that they're validating.
The DicomArchiveAnonymizer constructor seems reasonable, or in the module's Module class.
johnsaigle
removed
the
State: Needs work
| @@ -106,6 +167,9 @@ class Dicom_Archive extends \NDB_Menu_Filter | |||
| */ | |||
| function toJSON() | |||
| { | |||
| /* Ensure that the server is well configured to prevent displaying | |||
| * data that is not properly anonmyized. | |||
| */ | |||
There was a problem hiding this comment.
This comment is still here
johnsaigle
added
the
State: Needs work
johnsaigle
removed
the
State: Needs work
Brief summary of changes
While hunting down a PHP Warning I discovered that
dicomarchiveanonymizer.class.incdoes not do any validation on the config settings it uses. This results in empty values being sent topreg_matchwith possible consequences where DICOMs are not properly anonymized. I did not actually discover a case where this is true but my dummy imaging data is very limited.This PR adds some validation to
dicom_archive.class.inc. It will now refuse to show DICOM data when the regex config values are not properly configured.This resolves issue...
Should eliminate warnings like
PHP Warning: preg_match(): Empty regular expression in /var/www/loris/modules/dicom_archive/php/dicomarchiveanonymizer.class.inc on line 62, referer: https://jsaigle-dev.loris.ca/dicom_archive/To test this change...