# HG changeset patch # User Alain Mazy # Date 1659516590 -7200 # Node ID d4e5ca0c93070f51c54a52cd4dea743e16fa3a39 # Parent e6f26be401fa732fbd324aef7024a51a67a8d64f Fix the "Never" option of the "StorageAccessOnFind" that was sill accessing files (bug introduced in 1.11.0) diff -r e6f26be401fa -r d4e5ca0c9307 NEWS --- a/NEWS Tue Aug 02 11:38:31 2022 +0200 +++ b/NEWS Wed Aug 03 10:49:50 2022 +0200 @@ -6,6 +6,13 @@ * Added support for RGBA64 images in tools/create-dicom and /preview +Bug Fixes +--------- + +* Fix the "Never" option of the "StorageAccessOnFind" that was sill accessing + files (bug introduced in 1.11.0). + + Maintenance ----------- diff -r e6f26be401fa -r d4e5ca0c9307 OrthancServer/Sources/OrthancFindRequestHandler.cpp --- a/OrthancServer/Sources/OrthancFindRequestHandler.cpp Tue Aug 02 11:38:31 2022 +0200 +++ b/OrthancServer/Sources/OrthancFindRequestHandler.cpp Wed Aug 03 10:49:50 2022 +0200 @@ -49,7 +49,8 @@ const std::list& sequencesToReturn, const std::string& defaultPrivateCreator, const std::map& privateCreators, - const std::string& retrieveAet) + const std::string& retrieveAet, + bool allowStorageAccess) { ExpandedResource resource; std::set requestedTags; @@ -58,7 +59,7 @@ requestedTags.erase(DICOM_TAG_QUERY_RETRIEVE_LEVEL); // this is not part of the answer // reuse ExpandResource to get missing tags and computed tags (ModalitiesInStudy ...). This code is therefore shared between C-Find, tools/find, list-resources and QIDO-RS - context.ExpandResource(resource, publicId, mainDicomTags, instanceId, dicomAsJson, level, requestedTags, ExpandResourceDbFlags_IncludeMainDicomTags); + context.ExpandResource(resource, publicId, mainDicomTags, instanceId, dicomAsJson, level, requestedTags, ExpandResourceDbFlags_IncludeMainDicomTags, allowStorageAccess); DicomMap result; @@ -246,6 +247,7 @@ std::string defaultPrivateCreator_; // the private creator to use if the group is not defined in the query itself const std::map& privateCreators_; // the private creators defined in the query itself std::string retrieveAet_; + FindStorageAccessMode findStorageAccessMode_; public: LookupVisitor(DicomFindAnswers& answers, @@ -253,14 +255,16 @@ ResourceType level, const DicomMap& query, const std::list& sequencesToReturn, - const std::map& privateCreators) : + const std::map& privateCreators, + FindStorageAccessMode findStorageAccessMode) : answers_(answers), context_(context), level_(level), query_(query), queryAsArray_(query), sequencesToReturn_(sequencesToReturn), - privateCreators_(privateCreators) + privateCreators_(privateCreators), + findStorageAccessMode_(findStorageAccessMode) { answers_.SetComplete(false); @@ -308,7 +312,7 @@ const Json::Value* dicomAsJson) ORTHANC_OVERRIDE { AddAnswer(answers_, context_, publicId, instanceId, mainDicomTags, dicomAsJson, level_, queryAsArray_, sequencesToReturn_, - defaultPrivateCreator_, privateCreators_, retrieveAet_); + defaultPrivateCreator_, privateCreators_, retrieveAet_, IsStorageAccessAllowedForAnswers(findStorageAccessMode_)); } }; @@ -478,7 +482,7 @@ size_t limit = (level == ResourceType_Instance) ? maxInstances_ : maxResults_; - LookupVisitor visitor(answers, context_, level, *filteredInput, sequencesToReturn, privateCreators); + LookupVisitor visitor(answers, context_, level, *filteredInput, sequencesToReturn, privateCreators, context_.GetFindStorageAccessMode()); context_.Apply(visitor, lookup, level, 0 /* "since" is not relevant to C-FIND */, limit); } diff -r e6f26be401fa -r d4e5ca0c9307 OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp --- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp Tue Aug 02 11:38:31 2022 +0200 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp Wed Aug 03 10:49:50 2022 +0200 @@ -131,11 +131,12 @@ const std::list& resources, const std::map& instancesIds, // optional: the id of an instance for each found resource. const std::map >& resourcesMainDicomTags, // optional: all tags read from DB for a resource (current level and upper levels) - const std::map& resourcesDicomAsJson, // optional: the dicom-as-json for each resource + const std::map& resourcesDicomAsJson, // optional: the dicom-as-json for each resource ResourceType level, bool expand, DicomToJsonFormat format, - const std::set& requestedTags) + const std::set& requestedTags, + bool allowStorageAccess) { Json::Value answer = Json::arrayValue; @@ -145,7 +146,26 @@ if (expand) { Json::Value expanded; - if (context.ExpandResource(expanded, *resource, level, format, requestedTags)) + + std::map::const_iterator instanceId = instancesIds.find(*resource); + if (instanceId != instancesIds.end()) // if it is found in instancesIds, it is also in resourcesDicomAsJson and mainDicomTags + { + // reuse data already collected before (e.g during lookup) + std::map >::const_iterator mainDicomTags = resourcesMainDicomTags.find(*resource); + std::map::const_iterator dicomAsJson = resourcesDicomAsJson.find(*resource); + + context.ExpandResource(expanded, *resource, + *(mainDicomTags->second.get()), + instanceId->second, + dicomAsJson->second, + level, format, requestedTags, allowStorageAccess); + } + else + { + context.ExpandResource(expanded, *resource, level, format, requestedTags, allowStorageAccess); + } + + if (expanded.type() == Json::objectValue) { answer.append(expanded); } @@ -166,13 +186,14 @@ ResourceType level, bool expand, DicomToJsonFormat format, - const std::set& requestedTags) + const std::set& requestedTags, + bool allowStorageAccess) { std::map unusedInstancesIds; std::map > unusedResourcesMainDicomTags; - std::map unusedResourcesDicomAsJson; - - AnswerListOfResources(output, context, resources, unusedInstancesIds, unusedResourcesMainDicomTags, unusedResourcesDicomAsJson, level, expand, format, requestedTags); + std::map unusedResourcesDicomAsJson; + + AnswerListOfResources(output, context, resources, unusedInstancesIds, unusedResourcesMainDicomTags, unusedResourcesDicomAsJson, level, expand, format, requestedTags, allowStorageAccess); } @@ -235,7 +256,8 @@ AnswerListOfResources(call.GetOutput(), context, result, resourceType, call.HasArgument("expand"), OrthancRestApi::GetDicomFormat(call, DicomToJsonFormat_Human), - requestedTags); + requestedTags, + true /* allowStorageAccess */); } @@ -266,7 +288,7 @@ Json::Value json; if (OrthancRestApi::GetContext(call).ExpandResource( - json, call.GetUriComponent("id", ""), resourceType, format, requestedTags)) + json, call.GetUriComponent("id", ""), resourceType, format, requestedTags, true /* allowStorageAccess */)) { call.GetOutput().AnswerJson(json); } @@ -2848,17 +2870,19 @@ private: bool isComplete_; std::list resources_; + FindStorageAccessMode findStorageAccessMode_; // cache the data we used during lookup and that we could reuse when building the answers std::map instancesIds_; // the id of an instance for each found resource. std::map > resourcesMainDicomTags_; // all tags read from DB for a resource (current level and upper levels) - std::map resourcesDicomAsJson_; // the dicom-as-json for a resource + std::map resourcesDicomAsJson_; // the dicom-as-json for a resource DicomToJsonFormat format_; public: - explicit FindVisitor(DicomToJsonFormat format) : + explicit FindVisitor(DicomToJsonFormat format, FindStorageAccessMode findStorageAccessMode) : isComplete_(false), + findStorageAccessMode_(findStorageAccessMode), format_(format) { } @@ -2890,7 +2914,7 @@ bool expand, const std::set& requestedTags) const { - AnswerListOfResources(output, context, resources_, instancesIds_, resourcesMainDicomTags_, resourcesDicomAsJson_, level, expand, format_, requestedTags); + AnswerListOfResources(output, context, resources_, instancesIds_, resourcesMainDicomTags_, resourcesDicomAsJson_, level, expand, format_, requestedTags, IsStorageAccessAllowedForAnswers(findStorageAccessMode_)); } }; } @@ -3056,7 +3080,7 @@ } } - FindVisitor visitor(OrthancRestApi::GetDicomFormat(request, DicomToJsonFormat_Human)); + FindVisitor visitor(OrthancRestApi::GetDicomFormat(request, DicomToJsonFormat_Human), context.GetFindStorageAccessMode()); context.Apply(visitor, query, level, since, limit); visitor.Answer(call.GetOutput(), context, level, expand, requestedTags); } @@ -3119,7 +3143,7 @@ it = a.begin(); it != a.end(); ++it) { Json::Value resource; - if (OrthancRestApi::GetContext(call).ExpandResource(resource, *it, end, format, requestedTags)) + if (OrthancRestApi::GetContext(call).ExpandResource(resource, *it, end, format, requestedTags, true /* allowStorageAccess */)) { result.append(resource); } @@ -3238,7 +3262,7 @@ const DicomToJsonFormat format = OrthancRestApi::GetDicomFormat(call, DicomToJsonFormat_Human); Json::Value resource; - if (OrthancRestApi::GetContext(call).ExpandResource(resource, current, end, format, requestedTags)) + if (OrthancRestApi::GetContext(call).ExpandResource(resource, current, end, format, requestedTags, true /* allowStorageAccess */)) { call.GetOutput().AnswerJson(resource); } @@ -3645,7 +3669,7 @@ Json::Value item; std::set emptyRequestedTags; // not supported for bulk content - if (OrthancRestApi::GetContext(call).ExpandResource(item, *it, level, format, emptyRequestedTags)) + if (OrthancRestApi::GetContext(call).ExpandResource(item, *it, level, format, emptyRequestedTags, true /* allowStorageAccess */)) { if (metadata) { @@ -3670,7 +3694,7 @@ std::set emptyRequestedTags; // not supported for bulk content if (index.LookupResourceType(level, *it) && - OrthancRestApi::GetContext(call).ExpandResource(item, *it, level, format, emptyRequestedTags)) + OrthancRestApi::GetContext(call).ExpandResource(item, *it, level, format, emptyRequestedTags, true /* allowStorageAccess */)) { if (metadata) { diff -r e6f26be401fa -r d4e5ca0c9307 OrthancServer/Sources/OrthancWebDav.cpp --- a/OrthancServer/Sources/OrthancWebDav.cpp Tue Aug 02 11:38:31 2022 +0200 +++ b/OrthancServer/Sources/OrthancWebDav.cpp Wed Aug 03 10:49:50 2022 +0200 @@ -261,7 +261,7 @@ Json::Value resource; std::set emptyRequestedTags; // not supported for webdav - if (context_.ExpandResource(resource, publicId, level_, DicomToJsonFormat_Human, emptyRequestedTags)) + if (context_.ExpandResource(resource, publicId, level_, DicomToJsonFormat_Human, emptyRequestedTags, true /* allowStorageAccess */)) { if (success_) { diff -r e6f26be401fa -r d4e5ca0c9307 OrthancServer/Sources/ServerContext.cpp --- a/OrthancServer/Sources/ServerContext.cpp Tue Aug 02 11:38:31 2022 +0200 +++ b/OrthancServer/Sources/ServerContext.cpp Wed Aug 03 10:49:50 2022 +0200 @@ -1442,15 +1442,14 @@ // Optimization in Orthanc 1.5.1 - Don't read the full JSON from // the disk if only "main DICOM tags" are to be returned - std::unique_ptr dicomAsJson; + boost::shared_ptr dicomAsJson; bool hasOnlyMainDicomTags; DicomMap dicom; DicomMap allMainDicomTagsFromDB; - if (findStorageAccessMode_ == FindStorageAccessMode_DatabaseOnly || - findStorageAccessMode_ == FindStorageAccessMode_DiskOnAnswer || - fastLookup->HasOnlyMainDicomTags()) + if (!IsStorageAccessAllowedForAnswers(findStorageAccessMode_) + || fastLookup->HasOnlyMainDicomTags()) { // Case (1): The main DICOM tags, as stored in the database, // are sufficient to look for match @@ -1538,8 +1537,7 @@ } else { - if ((findStorageAccessMode_ == FindStorageAccessMode_DiskOnLookupAndAnswer || - findStorageAccessMode_ == FindStorageAccessMode_DiskOnAnswer) && + if (IsStorageAccessAllowedForAnswers(findStorageAccessMode_) && dicomAsJson.get() == NULL && isDicomAsJsonNeeded) { @@ -2336,13 +2334,14 @@ const std::string& publicId, ResourceType level, DicomToJsonFormat format, - const std::set& requestedTags) + const std::set& requestedTags, + bool allowStorageAccess) { std::string unusedInstanceId; Json::Value* unusedDicomAsJson = NULL; DicomMap unusedMainDicomTags; - return ExpandResource(target, publicId, unusedMainDicomTags, unusedInstanceId, unusedDicomAsJson, level, format, requestedTags); + return ExpandResource(target, publicId, unusedMainDicomTags, unusedInstanceId, unusedDicomAsJson, level, format, requestedTags, allowStorageAccess); } bool ServerContext::ExpandResource(Json::Value& target, @@ -2352,11 +2351,12 @@ const Json::Value* dicomAsJson, // optional: the dicom-as-json for the resource (if already available) ResourceType level, DicomToJsonFormat format, - const std::set& requestedTags) + const std::set& requestedTags, + bool allowStorageAccess) { ExpandedResource resource; - if (ExpandResource(resource, publicId, mainDicomTags, instanceId, dicomAsJson, level, requestedTags, ExpandResourceDbFlags_Default)) + if (ExpandResource(resource, publicId, mainDicomTags, instanceId, dicomAsJson, level, requestedTags, ExpandResourceDbFlags_Default, allowStorageAccess)) { SerializeExpandedResource(target, resource, format, requestedTags); return true; @@ -2372,7 +2372,8 @@ const Json::Value* dicomAsJson, // optional: the dicom-as-json for the resource (if already available) ResourceType level, const std::set& requestedTags, - ExpandResourceDbFlags expandFlags) + ExpandResourceDbFlags expandFlags, + bool allowStorageAccess) { // first try to get the tags from what is already available @@ -2420,7 +2421,8 @@ } // possibly merge missing requested tags from dicom-as-json - if (!resource.missingRequestedTags_.empty() && !DicomMap::HasOnlyComputedTags(resource.missingRequestedTags_)) + if (allowStorageAccess + && !resource.missingRequestedTags_.empty() && !DicomMap::HasOnlyComputedTags(resource.missingRequestedTags_)) { OrthancConfiguration::ReaderLock lock; if (lock.GetConfiguration().IsWarningEnabled(Warnings_001_TagsBeingReadFromStorage)) diff -r e6f26be401fa -r d4e5ca0c9307 OrthancServer/Sources/ServerContext.h --- a/OrthancServer/Sources/ServerContext.h Tue Aug 02 11:38:31 2022 +0200 +++ b/OrthancServer/Sources/ServerContext.h Wed Aug 03 10:49:50 2022 +0200 @@ -545,7 +545,8 @@ const std::string& publicId, ResourceType level, DicomToJsonFormat format, - const std::set& requestedTags); + const std::set& requestedTags, + bool allowStorageAccess); bool ExpandResource(Json::Value& target, const std::string& publicId, @@ -554,7 +555,8 @@ const Json::Value* dicomAsJson, // optional: the dicom-as-json for the resource ResourceType level, DicomToJsonFormat format, - const std::set& requestedTags); + const std::set& requestedTags, + bool allowStorageAccess); bool ExpandResource(ExpandedResource& target, const std::string& publicId, @@ -563,7 +565,12 @@ const Json::Value* dicomAsJson, // optional: the dicom-as-json for the resource ResourceType level, const std::set& requestedTags, - ExpandResourceDbFlags expandFlags); + ExpandResourceDbFlags expandFlags, + bool allowStorageAccess); + FindStorageAccessMode GetFindStorageAccessMode() const + { + return findStorageAccessMode_; + } }; } diff -r e6f26be401fa -r d4e5ca0c9307 OrthancServer/Sources/ServerEnumerations.cpp --- a/OrthancServer/Sources/ServerEnumerations.cpp Tue Aug 02 11:38:31 2022 +0200 +++ b/OrthancServer/Sources/ServerEnumerations.cpp Wed Aug 03 10:49:50 2022 +0200 @@ -208,6 +208,15 @@ } } + bool IsStorageAccessAllowedForAnswers(FindStorageAccessMode mode) + { + return mode != FindStorageAccessMode_DatabaseOnly; + } + + bool IsStorageAccessAllowedForLookup(FindStorageAccessMode mode) + { + return mode == FindStorageAccessMode_DiskOnLookupAndAnswer; + } BuiltinDecoderTranscoderOrder StringToBuiltinDecoderTranscoderOrder(const std::string& value) { diff -r e6f26be401fa -r d4e5ca0c9307 OrthancServer/Sources/ServerEnumerations.h --- a/OrthancServer/Sources/ServerEnumerations.h Tue Aug 02 11:38:31 2022 +0200 +++ b/OrthancServer/Sources/ServerEnumerations.h Wed Aug 03 10:49:50 2022 +0200 @@ -217,6 +217,10 @@ FindStorageAccessMode StringToFindStorageAccessMode(const std::string& str); + bool IsStorageAccessAllowedForAnswers(FindStorageAccessMode mode); + + bool IsStorageAccessAllowedForLookup(FindStorageAccessMode mode); + BuiltinDecoderTranscoderOrder StringToBuiltinDecoderTranscoderOrder(const std::string& str); Verbosity StringToVerbosity(const std::string& str);