diff --git a/Applications/Cxx/gdcmclean.cxx b/Applications/Cxx/gdcmclean.cxx index a618fd99d..c78c95461 100644 --- a/Applications/Cxx/gdcmclean.cxx +++ b/Applications/Cxx/gdcmclean.cxx @@ -138,6 +138,10 @@ static void PrintHelp() { std::cout << " --preserve-illegal Whether or " "not preserve illegal attributes (eg. group 0003...)." << std::endl; + std::cout + << " --empty-when-scrub-fails Fallback to " + "emptying any attribute for which scrub operation failed." + << std::endl; std::cout << "General Options:" << std::endl; std::cout << " -V --verbose more verbose (warning+error)." << std::endl; @@ -171,6 +175,7 @@ int main(int argc, char *argv[]) { int preserveAllMissingPrivateCreator = 0; int preserveAllGroupLength = 0; int preserveAllIllegal = 0; + int emptyWhenScrubFails = 0; std::vector empty_dpaths; std::vector remove_dpaths; std::vector scrub_dpaths; @@ -196,7 +201,7 @@ int main(int argc, char *argv[]) { {"recursive", no_argument, nullptr, 'r'}, {"empty", required_argument, &empty_tag, 1}, // 3 {"remove", required_argument, &remove_tag, 1}, // 4 - {"scrub", required_argument, &scrub_tag, 1}, // 5 + {"scrub", required_argument, &scrub_tag, 1}, // 5 {"preserve", required_argument, &preserve_tag, 1}, // 5 {"continue", no_argument, nullptr, 'c'}, {"skip-meta", 0, &skipmeta, 1}, // should I document this one ? @@ -204,6 +209,7 @@ int main(int argc, char *argv[]) { &preserveAllMissingPrivateCreator, 1}, // {"preserve-group-length", 0, &preserveAllGroupLength, 1}, // {"preserve-illegal", 0, &preserveAllIllegal, 1}, // + {"empty-when-scrub-fails", 0, &emptyWhenScrubFails, 1}, // {"verbose", no_argument, nullptr, 'V'}, {"warning", no_argument, nullptr, 'W'}, @@ -437,6 +443,7 @@ int main(int argc, char *argv[]) { cleaner.RemoveAllMissingPrivateCreator(!preserveAllMissingPrivateCreator); cleaner.RemoveAllGroupLength(!preserveAllGroupLength); cleaner.RemoveAllIllegal(!preserveAllIllegal); + cleaner.EmptyWhenScrubFails(!!emptyWhenScrubFails); // Preserve for (std::vector::const_iterator it = preserve_dpaths.begin(); it != preserve_dpaths.end(); ++it) { diff --git a/Applications/Cxx/gdcmdump.cxx b/Applications/Cxx/gdcmdump.cxx index 9cc80ac78..aba27a7bd 100644 --- a/Applications/Cxx/gdcmdump.cxx +++ b/Applications/Cxx/gdcmdump.cxx @@ -1137,8 +1137,9 @@ static int PrintCSA(const std::string & filename) int ret = 0; if( ds.FindDataElement( t1 ) ) { - csa.LoadFromDataElement( ds.GetDataElement( t1 ) ); - csa.Print( std::cout ); + if( csa.LoadFromDataElement( ds.GetDataElement( t1 ) ) ) + csa.Print( std::cout ); + else ret = 1; found = true; if( csa.GetFormat() == gdcm::CSAHeader::ZEROED_OUT ) { @@ -1156,8 +1157,9 @@ static int PrintCSA(const std::string & filename) } if( ds.FindDataElement( t2 ) ) { - csa.LoadFromDataElement( ds.GetDataElement( t2 ) ); - csa.Print( std::cout ); + if( csa.LoadFromDataElement( ds.GetDataElement( t2 ) ) ) + csa.Print( std::cout ); + else ret = 1; found = true; if( csa.GetFormat() == gdcm::CSAHeader::ZEROED_OUT ) { @@ -1175,8 +1177,9 @@ static int PrintCSA(const std::string & filename) } if( ds.FindDataElement( t3 ) ) { - csa.LoadFromDataElement( ds.GetDataElement( t3 ) ); - csa.Print( std::cout ); + if( csa.LoadFromDataElement( ds.GetDataElement( t3 ) ) ) + csa.Print( std::cout ); + else ret = 1; found = true; if( csa.GetFormat() == gdcm::CSAHeader::ZEROED_OUT ) { diff --git a/Source/DataStructureAndEncodingDefinition/gdcmCSAHeader.cxx b/Source/DataStructureAndEncodingDefinition/gdcmCSAHeader.cxx index ad944ac64..3fe9075c0 100644 --- a/Source/DataStructureAndEncodingDefinition/gdcmCSAHeader.cxx +++ b/Source/DataStructureAndEncodingDefinition/gdcmCSAHeader.cxx @@ -910,7 +910,8 @@ bool check_mapping(uint32_t syngodt, const char *vr) { static const unsigned int max = sizeof(mapping) / sizeof(equ); const equ *p = mapping; - assert( syngodt <= mapping[max-1].syngodt ); (void)max; + if( syngodt > mapping[max-1].syngodt ) return false; + assert( syngodt <= mapping[max-1].syngodt ); while(p->syngodt < syngodt ) { //std::cout << "mapping:" << p->vr << std::endl; @@ -1098,6 +1099,11 @@ bool CSAHeader::LoadFromDataElement(DataElement const &de) char vr[4]; ss.read(vr, 4); // In dataset without magic signature (OLD FORMAT) vr[3] is garbage... + if( vr[2] != 0 ) + { + gdcmErrorMacro( "Garbage data. Stopping CSA parsing." ); + return false; + } assert( /*vr[3] == 0 &&*/ vr[2] == 0 ); csael.SetVR( VR::GetVRTypeFromFile(vr) ); //std::cout << "VR " << vr << ", "; @@ -1131,8 +1137,10 @@ bool CSAHeader::LoadFromDataElement(DataElement const &de) uint32_t item_xx[4]; ss.read((char*)&item_xx, 4*sizeof(uint32_t)); SwapperNoOp::SwapArray(item_xx,4); + if( item_xx[2] != 77 && item_xx[2] != 205 ) return false; assert( item_xx[2] == 77 || item_xx[2] == 205 ); uint32_t len = item_xx[1]; // 2nd element + if( item_xx[0] != item_xx[1] || item_xx[1] != item_xx[3] ) return false; assert( item_xx[0] == item_xx[1] && item_xx[1] == item_xx[3] ); if( len ) { diff --git a/Source/MediaStorageAndFileFormat/gdcmCleaner.cxx b/Source/MediaStorageAndFileFormat/gdcmCleaner.cxx index e16001526..0d0ffe28b 100644 --- a/Source/MediaStorageAndFileFormat/gdcmCleaner.cxx +++ b/Source/MediaStorageAndFileFormat/gdcmCleaner.cxx @@ -538,10 +538,12 @@ struct Cleaner::impl { bool AllMissingPrivateCreator; bool AllGroupLength; bool AllIllegal; + bool WhenScrubFails; impl() : AllMissingPrivateCreator(true), AllGroupLength(true), - AllIllegal(true) {} + AllIllegal(true), + WhenScrubFails(false) {} enum ACTION { NONE, EMPTY, REMOVE, SCRUB }; enum ACTION ComputeAction(File const &file, DataSet &ds, @@ -668,6 +670,10 @@ struct Cleaner::impl { bool RemoveMissingPrivateCreator(Tag const & /*t*/) { return false; } void RemoveAllGroupLength(bool remove) { AllGroupLength = remove; } void RemoveAllIllegal(bool remove) { AllIllegal = remove; } + void EmptyWhenScrubFails(bool empty) { WhenScrubFails = empty; } + + bool CleanCSAImage(DataSet &ds, const DataElement &de); + bool CleanCSASeries(DataSet &ds, const DataElement &de); }; static VR ComputeDictVR(File &file, DataSet &ds, DataElement const &de) { @@ -840,7 +846,7 @@ static inline bool bs_is_signature(const ByteValue *bv, const char *str) { return false; } -static bool CleanCSAImage(DataSet &ds, const DataElement &de) { +bool Cleaner::impl::CleanCSAImage(DataSet &ds, const DataElement &de) { const ByteValue *bv = de.GetByteValue(); // fast path: if (!bv) return true; @@ -888,6 +894,13 @@ static bool CleanCSAImage(DataSet &ds, const DataElement &de) { gdcmDebugMacro("Zero-out CSA header"); return true; } + // fallback logic: + if (WhenScrubFails && is_signature(bv, sv10)) { + // so SV10 header has been identified, but we failed to 'scrub', let's + // empty it: + ds.Replace(clean); + return true; + } gdcmErrorMacro("Failure to call CleanCSAImage"); return false; } @@ -900,7 +913,7 @@ static bool CleanCSAImage(DataSet &ds, const DataElement &de) { return true; } -static bool CleanCSASeries(DataSet &ds, const DataElement &de) { +bool Cleaner::impl::CleanCSASeries(DataSet &ds, const DataElement &de) { const ByteValue *bv = de.GetByteValue(); // fast path: if (!bv) return true; @@ -944,6 +957,13 @@ static bool CleanCSASeries(DataSet &ds, const DataElement &de) { gdcmDebugMacro("Zero-out CSA header"); return true; } + // fallback logic: + if (WhenScrubFails && is_signature(bv, sv10)) { + // so SV10 header has been identified, but we failed to 'scrub', let's + // empty it: + ds.Replace(clean); + return true; + } gdcmErrorMacro("Failure to call CleanCSASeries"); return false; } @@ -1065,8 +1085,8 @@ static bool IsDPathInSet(std::set const &aset, DPath const dpath) { } Cleaner::impl::ACTION Cleaner::impl::ComputeAction( - File const & /*file*/, DataSet &ds, const DataElement &de, VR const &ref_dict_vr, - const std::string &tag_path) { + File const & /*file*/, DataSet &ds, const DataElement &de, + VR const &ref_dict_vr, const std::string &tag_path) { const Tag &tag = de.GetTag(); // Group Length & Illegal cannot be preserved so it is safe to do them now: if (tag.IsGroupLength()) { @@ -1302,6 +1322,9 @@ void Cleaner::RemoveAllGroupLength(bool remove) { pimpl->RemoveAllGroupLength(remove); } void Cleaner::RemoveAllIllegal(bool remove) { pimpl->RemoveAllIllegal(remove); } +void Cleaner::EmptyWhenScrubFails(bool empty) { + pimpl->EmptyWhenScrubFails(empty); +} bool Cleaner::Clean() { DataSet &ds = F->GetDataSet(); diff --git a/Source/MediaStorageAndFileFormat/gdcmCleaner.h b/Source/MediaStorageAndFileFormat/gdcmCleaner.h index 4d8c351ab..bf2ad5443 100644 --- a/Source/MediaStorageAndFileFormat/gdcmCleaner.h +++ b/Source/MediaStorageAndFileFormat/gdcmCleaner.h @@ -65,6 +65,9 @@ class GDCM_EXPORT Cleaner : public Subject { /// Should I remove all illegal attribute. Default: true void RemoveAllIllegal(bool remove); + /// Should I empty instead of scrub upon failure + void EmptyWhenScrubFails(bool empty); + /// main loop bool Clean(); diff --git a/Utilities/gdcmext/csa.c b/Utilities/gdcmext/csa.c index bbab6b391..517de950e 100644 --- a/Utilities/gdcmext/csa.c +++ b/Utilities/gdcmext/csa.c @@ -89,8 +89,15 @@ static void setup_buffer(struct app *self, void *output, const void *input, self->out->write = stream_write; } +//#define CSA_DEBUG + +#ifdef CSA_DEBUG +#define ERROR_RETURN(X, Y) \ + if ((X) != (Y)) assert(0); +#else #define ERROR_RETURN(X, Y) \ if ((X) != (Y)) return false +#endif static size_t fread_mirror(void *ptr, size_t size, size_t nmemb, struct app *self) { @@ -214,16 +221,18 @@ static bool read_info(struct app *self, struct csa_info *i) { s = fread_mirror(&i->vm, sizeof i->vm, 1, self); ERROR_RETURN(s, 1); // vm == 115301884 seems to be ok... - assert(i->vm < 0x6df5dfd); + ERROR_RETURN(i->vm < 0x6df5dfd, true); // vr s = fread_mirror(i->vr, sizeof *i->vr, sizeof i->vr / sizeof *i->vr, self); ERROR_RETURN(s, 4); { const char *s = i->vr; - assert(s[0] >= 'A' && s[0] <= 'Z'); - assert(s[1] >= 'A' && s[1] <= 'Z'); - assert(s[2] == 0); - if (self->csa_type == SV10) assert(s[3] == 0); + ERROR_RETURN(s[0] >= 'A' && s[0] <= 'Z', true); + ERROR_RETURN(s[1] >= 'A' && s[1] <= 'Z', true); + ERROR_RETURN(s[2], 0); + if (self->csa_type == SV10) { + ERROR_RETURN(s[3], 0); + } } // syngodt (signed) s = fread_mirror(&i->syngodt, sizeof i->syngodt, 1, self); @@ -250,15 +259,15 @@ static bool read_data(struct app *self, struct csa_item_data *d) { uint32_t unused; s = fread_mirror(&unused, sizeof unused, 1, self); ERROR_RETURN(s, 1); - assert(unused == d->len || unused == 0x4d || unused == 0xcd); + ERROR_RETURN(unused == d->len || unused == 0x4d || unused == 0xcd, true); } if (d->len != 0) { const uint32_t padding_len = (4 - d->len % 4) % 4; const uint32_t padded_len = ((d->len + 3u) / 4u) * 4u; // (len + 3) & ~0x03 assert(padded_len == d->len + padding_len); // programmer error - d->buffer = (char *)realloc(d->buffer, padded_len); assert(padded_len != 0); + d->buffer = (char *)realloc(d->buffer, padded_len); ERROR_RETURN(d->buffer != NULL, true); s = fread_mirror(d->buffer, sizeof *d->buffer, padded_len, self); ERROR_RETURN(s, padded_len);