Mercurial > hg > orthanc
changeset 4737:979ae3ea3381
DANGEROUS commit: Anonymization is now also applied to nested sequences
| author | Sebastien Jodogne <s.jodogne@gmail.com> |
|---|---|
| date | Tue, 06 Jul 2021 08:12:26 +0200 |
| parents | bf852fd773b7 |
| children | 9ce946d28e41 |
| files | NEWS OrthancFramework/Sources/DicomParsing/DicomModification.cpp OrthancFramework/Sources/DicomParsing/DicomModification.h OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp OrthancFramework/Sources/DicomParsing/ITagVisitor.h OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp |
| diffstat | 8 files changed, 191 insertions(+), 82 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Mon Jul 05 17:50:58 2021 +0200 +++ b/NEWS Tue Jul 06 08:12:26 2021 +0200 @@ -1,6 +1,11 @@ Pending changes in the mainline =============================== +General +------- + +* Anonymization is now also applied to nested sequences + REST API --------
--- a/OrthancFramework/Sources/DicomParsing/DicomModification.cpp Mon Jul 05 17:50:58 2021 +0200 +++ b/OrthancFramework/Sources/DicomParsing/DicomModification.cpp Tue Jul 06 08:12:26 2021 +0200 @@ -44,6 +44,16 @@ namespace Orthanc { + namespace + { + enum TagOperation + { + TagOperation_Keep, + TagOperation_Remove + }; + } + + DicomModification::DicomTagRange::DicomTagRange(uint16_t groupFrom, uint16_t groupTo, uint16_t elementFrom, @@ -76,7 +86,57 @@ return (that_.IsCleared(tag) || that_.IsRemoved(tag) || that_.IsReplaced(tag)); - } + } + + bool IsKeptSequence(const std::vector<DicomTag>& parentTags, + const std::vector<size_t>& parentIndexes, + const DicomTag& tag) + { + for (DicomModification::ListOfPaths::const_iterator + it = that_.keepSequences_.begin(); it != that_.keepSequences_.end(); ++it) + { + if (DicomPath::IsMatch(*it, parentTags, parentIndexes, tag)) + { + return true; + } + } + + return false; + } + + Action GetDefaultAction(const std::vector<DicomTag>& parentTags, + const std::vector<size_t>& parentIndexes, + const DicomTag& tag) + { + if (parentTags.empty() || + !that_.isAnonymization_) + { + // Don't interfere with first-level tags or with modification + return Action_None; + } + else if (IsKeptSequence(parentTags, parentIndexes, tag)) + { + return Action_None; + } + else if (that_.ArePrivateTagsRemoved() && + tag.IsPrivate()) + { + // New in Orthanc 1.9.5 + // https://groups.google.com/g/orthanc-users/c/l1mcYCC2u-k/m/jOdGYuagAgAJ + return Action_Remove; + } + else if (that_.IsCleared(tag) || + that_.IsRemoved(tag)) + { + // New in Orthanc 1.9.5 + // https://groups.google.com/g/orthanc-users/c/l1mcYCC2u-k/m/jOdGYuagAgAJ + return Action_Remove; + } + else + { + return Action_None; + } + } public: explicit RelationshipsVisitor(DicomModification& that) : @@ -89,14 +149,15 @@ const DicomTag& tag, ValueRepresentation vr) ORTHANC_OVERRIDE { - return Action_None; + return GetDefaultAction(parentTags, parentIndexes, tag); } - virtual Action VisitEmptySequence(const std::vector<DicomTag>& parentTags, - const std::vector<size_t>& parentIndexes, - const DicomTag& tag) ORTHANC_OVERRIDE + virtual Action VisitSequence(const std::vector<DicomTag>& parentTags, + const std::vector<size_t>& parentIndexes, + const DicomTag& tag, + size_t countItems) ORTHANC_OVERRIDE { - return Action_None; + return GetDefaultAction(parentTags, parentIndexes, tag); } virtual Action VisitBinary(const std::vector<DicomTag>& parentTags, @@ -106,7 +167,7 @@ const void* data, size_t size) ORTHANC_OVERRIDE { - return Action_None; + return GetDefaultAction(parentTags, parentIndexes, tag); } virtual Action VisitIntegers(const std::vector<DicomTag>& parentTags, @@ -115,7 +176,7 @@ ValueRepresentation vr, const std::vector<int64_t>& values) ORTHANC_OVERRIDE { - return Action_None; + return GetDefaultAction(parentTags, parentIndexes, tag); } virtual Action VisitDoubles(const std::vector<DicomTag>& parentTags, @@ -124,7 +185,7 @@ ValueRepresentation vr, const std::vector<double>& value) ORTHANC_OVERRIDE { - return Action_None; + return GetDefaultAction(parentTags, parentIndexes, tag); } virtual Action VisitAttributes(const std::vector<DicomTag>& parentTags, @@ -132,7 +193,7 @@ const DicomTag& tag, const std::vector<DicomTag>& value) ORTHANC_OVERRIDE { - return Action_None; + return GetDefaultAction(parentTags, parentIndexes, tag); } virtual Action VisitString(std::string& newValue, @@ -187,18 +248,30 @@ { // We are within a sequence - if (!that_.keepSequences_.empty()) + if (IsKeptSequence(parentTags, parentIndexes, tag)) { // New in Orthanc 1.9.4 - Solves issue LSD-629 - DicomPath path(parentTags, parentIndexes, tag); - - for (ListOfPaths::const_iterator it = that_.keepSequences_.begin(); - it != that_.keepSequences_.end(); ++it) + return Action_None; + } + + if (that_.isAnonymization_) + { + // New in Orthanc 1.9.5, similar to "GetDefaultAction()" + // https://groups.google.com/g/orthanc-users/c/l1mcYCC2u-k/m/jOdGYuagAgAJ + if (that_.ArePrivateTagsRemoved() && + tag.IsPrivate()) { - if (DicomPath::IsMatch(*it, path)) - { - return Action_None; - } + return Action_Remove; + } + else if (that_.IsRemoved(tag)) + { + return Action_Remove; + } + else if (that_.IsCleared(tag)) + { + // This is different from "GetDefaultAction()", because we know how to clear string tags + newValue.clear(); + return Action_Replace; } } @@ -1106,7 +1179,7 @@ static void ParseListOfTags(DicomModification& target, const Json::Value& query, - DicomModification::TagOperation operation, + TagOperation operation, bool force) { if (!query.isArray()) @@ -1131,18 +1204,18 @@ { throw OrthancException(ErrorCode_BadRequest, "Marking tag \"" + name + "\" as to be " + - (operation == DicomModification::TagOperation_Keep ? "kept" : "removed") + + (operation == TagOperation_Keep ? "kept" : "removed") + " requires the \"Force\" option to be set to true"); } switch (operation) { - case DicomModification::TagOperation_Keep: + case TagOperation_Keep: target.Keep(path); LOG(TRACE) << "Keep: " << name << " = " << path.Format(); break; - case DicomModification::TagOperation_Remove: + case TagOperation_Remove: target.Remove(path); LOG(TRACE) << "Remove: " << name << " = " << path.Format(); break;
--- a/OrthancFramework/Sources/DicomParsing/DicomModification.h Mon Jul 05 17:50:58 2021 +0200 +++ b/OrthancFramework/Sources/DicomParsing/DicomModification.h Tue Jul 06 08:12:26 2021 +0200 @@ -39,12 +39,6 @@ **/ public: - enum TagOperation - { - TagOperation_Keep, - TagOperation_Remove - }; - class IDicomIdentifierGenerator : public boost::noncopyable { public:
--- a/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp Mon Jul 05 17:50:58 2021 +0200 +++ b/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp Tue Jul 06 08:12:26 2021 +0200 @@ -382,11 +382,13 @@ ITagVisitor::Action - DicomWebJsonVisitor::VisitEmptySequence(const std::vector<DicomTag>& parentTags, - const std::vector<size_t>& parentIndexes, - const DicomTag& tag) + DicomWebJsonVisitor::VisitSequence(const std::vector<DicomTag>& parentTags, + const std::vector<size_t>& parentIndexes, + const DicomTag& tag, + size_t countItems) { - if (tag.GetElement() != 0x0000) + if (countItems == 0 && + tag.GetElement() != 0x0000) { Json::Value& node = CreateNode(parentTags, parentIndexes, tag); node[KEY_VR] = EnumerationToString(ValueRepresentation_Sequence);
--- a/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h Mon Jul 05 17:50:58 2021 +0200 +++ b/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h Tue Jul 06 08:12:26 2021 +0200 @@ -91,9 +91,10 @@ ValueRepresentation vr) ORTHANC_OVERRIDE; - virtual Action VisitEmptySequence(const std::vector<DicomTag>& parentTags, - const std::vector<size_t>& parentIndexes, - const DicomTag& tag) + virtual Action VisitSequence(const std::vector<DicomTag>& parentTags, + const std::vector<size_t>& parentIndexes, + const DicomTag& tag, + size_t countItems) ORTHANC_OVERRIDE; virtual Action VisitBinary(const std::vector<DicomTag>& parentTags,
--- a/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp Mon Jul 05 17:50:58 2021 +0200 +++ b/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp Tue Jul 06 08:12:26 2021 +0200 @@ -2415,6 +2415,20 @@ evr = EVR_UN; } + if (evr == EVR_UN) + { + // New in Orthanc 1.9.5 + DictionaryLocker locker; + + const DcmDictEntry* entry = locker->findEntry(element.getTag().getXTag(), + element.getTag().getPrivateCreator()); + + if (entry != NULL) + { + evr = entry->getEVR(); + } + } + const ValueRepresentation vr = FromDcmtkBridge::Convert(evr); @@ -2846,41 +2860,38 @@ // etc. are not." The following dynamic_cast is thus OK. DcmSequenceOfItems& sequence = dynamic_cast<DcmSequenceOfItems&>(element); - if (sequence.card() == 0) + ITagVisitor::Action action = visitor.VisitSequence(parentTags, parentIndexes, tag, sequence.card()); + + switch (action) { - ITagVisitor::Action action = visitor.VisitEmptySequence(parentTags, parentIndexes, tag); - - switch (action) - { - case ITagVisitor::Action_None: - return true; - - case ITagVisitor::Action_Remove: - return false; - - case ITagVisitor::Action_Replace: - throw OrthancException(ErrorCode_NotImplemented, "Iterator cannot replace sequences"); - - default: - throw OrthancException(ErrorCode_ParameterOutOfRange); - } + case ITagVisitor::Action_None: + if (sequence.card() != 0) // Minor optimization to avoid creating "tags" and "indexes" if not needed + { + std::vector<DicomTag> tags = parentTags; + std::vector<size_t> indexes = parentIndexes; + tags.push_back(tag); + indexes.push_back(0); + + for (unsigned long i = 0; i < sequence.card(); i++) + { + indexes.back() = static_cast<size_t>(i); + DcmItem* child = sequence.getItem(i); + ApplyVisitorToDataset(*child, visitor, tags, indexes, encoding, hasCodeExtensions); + } + } + + return true; // Keep + + case ITagVisitor::Action_Remove: + return false; + + case ITagVisitor::Action_Replace: + throw OrthancException(ErrorCode_NotImplemented, "Iterator cannot replace sequences"); + + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); } - else - { - std::vector<DicomTag> tags = parentTags; - std::vector<size_t> indexes = parentIndexes; - tags.push_back(tag); - indexes.push_back(0); - - for (unsigned long i = 0; i < sequence.card(); i++) - { - indexes.back() = static_cast<size_t>(i); - DcmItem* child = sequence.getItem(i); - ApplyVisitorToDataset(*child, visitor, tags, indexes, encoding, hasCodeExtensions); - } - - return true; // Keep - } + } }
--- a/OrthancFramework/Sources/DicomParsing/ITagVisitor.h Mon Jul 05 17:50:58 2021 +0200 +++ b/OrthancFramework/Sources/DicomParsing/ITagVisitor.h Tue Jul 06 08:12:26 2021 +0200 @@ -51,9 +51,10 @@ ValueRepresentation vr) = 0; // SQ - can return "Remove" or "None" - virtual Action VisitEmptySequence(const std::vector<DicomTag>& parentTags, - const std::vector<size_t>& parentIndexes, - const DicomTag& tag) = 0; + virtual Action VisitSequence(const std::vector<DicomTag>& parentTags, + const std::vector<size_t>& parentIndexes, + const DicomTag& tag, + size_t countItems) = 0; // SL, SS, UL, US - can return "Remove" or "None" virtual Action VisitIntegers(const std::vector<DicomTag>& parentTags,
--- a/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp Mon Jul 05 17:50:58 2021 +0200 +++ b/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp Tue Jul 06 08:12:26 2021 +0200 @@ -2714,7 +2714,10 @@ ASSERT_NE("1.2.840.113619.2.176.2025.1499492.7040.1171286241.719", vv1[REF_IM_SEQ][0][REF_SOP_INSTANCE].asString()); ASSERT_NE("1.2.840.113619.2.176.2025.1499492.7040.1171286241.726", vv1[REF_IM_SEQ][1][REF_SOP_INSTANCE].asString()); ASSERT_NE("1.2.840.113704.1.111.7016.1342451220.40", vv1[REL_SERIES_SEQ][0][STUDY_INSTANCE_UID].asString()); - ASSERT_EQ("WORLD", vv1[REL_SERIES_SEQ][0][PURPOSE_CODE_SEQ][0][SERIES_DESCRIPTION].asString()); + + // Contrarily to Orthanc 1.9.4, the "SERIES_DESCRIPTION" is also removed from nested sequences + ASSERT_EQ(1u, vv1[REL_SERIES_SEQ][0][PURPOSE_CODE_SEQ][0].size()); + ASSERT_EQ("122403", vv1[REL_SERIES_SEQ][0][PURPOSE_CODE_SEQ][0]["0008,0100"].asString()); } { @@ -2774,17 +2777,35 @@ } } - virtual Action VisitEmptySequence(const std::vector<DicomTag>& parentTags, - const std::vector<size_t>& parentIndexes, - const DicomTag& tag) ORTHANC_OVERRIDE + virtual Action VisitSequence(const std::vector<DicomTag>& parentTags, + const std::vector<size_t>& parentIndexes, + const DicomTag& tag, + size_t countItems) ORTHANC_OVERRIDE { seen_ |= (1 << 1); - if (parentTags.size() == 1u && - parentIndexes.size() == 1u && - parentTags[0] == DICOM_TAG_REFERENCED_IMAGE_SEQUENCE && - parentIndexes[0] == 0u && - DcmTagKey(tag.GetGroup(), tag.GetElement()) == DCM_ReferencedPatientSequence) + if (parentTags.size() == 0u && + parentIndexes.size() == 0u && + tag == DICOM_TAG_REFERENCED_IMAGE_SEQUENCE && + countItems == 1) + { + return Action_None; + } + else if (parentTags.size() == 1u && + parentIndexes.size() == 1u && + parentTags[0] == DICOM_TAG_REFERENCED_IMAGE_SEQUENCE && + parentIndexes[0] == 0u && + countItems == 0 && + DcmTagKey(tag.GetGroup(), tag.GetElement()) == DCM_ReferencedPatientSequence) + { + return Action_Remove; + } + else if (parentTags.size() == 1u && + parentIndexes.size() == 1u && + parentTags[0] == DICOM_TAG_REFERENCED_IMAGE_SEQUENCE && + parentIndexes[0] == 0u && + countItems == 1 && + DcmTagKey(tag.GetGroup(), tag.GetElement()) == DCM_ReferencedStudySequence) { return Action_Remove; } @@ -2914,7 +2935,8 @@ v["ReferencedSOPClassUID"] = "1.2.840.10008.5.1.4.1.1.4"; v["ReferencedImageSequence"][0]["ReferencedSOPClassUID"] = "1.2.840.10008.5.1.4.1.1.4"; v["ReferencedImageSequence"][0]["ReferencedSOPInstanceUID"] = "1.2.840.113619.2.176.2025.1499492.7040.1171286241.719"; - v["ReferencedImageSequence"][0]["ReferencedPatientSequence"] = Json::arrayValue; // Empty sequence + v["ReferencedImageSequence"][0]["ReferencedPatientSequence"] = Json::arrayValue; // Empty nested sequence + v["ReferencedImageSequence"][0]["ReferencedStudySequence"][0]["PatientID"] = "Hello"; // Non-empty nested sequence v["ReferencedImageSequence"][0]["0011,1311"] = "abcd"; // Binary dicom.reset(ParsedDicomFile::CreateFromJson(v, DicomFromJsonFlags_None, "PrivateCreator"));