Skip to content

Commit

Permalink
Bad gzip compression fixed.
Browse files Browse the repository at this point in the history
Unfortunately, the file that reproduces this error is classified for now. So no tests for now :(
  • Loading branch information
rafamanzo committed Mar 13, 2014
1 parent a252ee9 commit b670ec4
Showing 1 changed file with 23 additions and 2 deletions.
25 changes: 23 additions & 2 deletions lib/nifti/n_read.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ class NRead
attr_reader :image_rubyarray
# A narray of image values reshapred to image dimensions
attr_reader :image_narray
# Flag for GZip
attr_reader :gzip

# Valid Magic codes for the NIFTI Header
MAGIC = %w{ni1 n+1}
Expand Down Expand Up @@ -43,6 +45,7 @@ def initialize(source=nil, options={})
options[:image] = true if options[:narray]
@msg = []
@success = false
@gzip = false
set_stream(source, options)
parse_header(options)

Expand Down Expand Up @@ -100,7 +103,6 @@ def set_stream(source, options)
else
# Extract the content of the file to a binary string:
@str = @file.read
@file.close
end
end

Expand All @@ -111,7 +113,24 @@ def set_stream(source, options)

# Parse the NIFTI Header.
def parse_header(options = {})
check_header
begin
check_header
# There is a specific gziped file (unfortunately it is confidential) which calling the read method returns less bytes than the expected. Failling back to byte per byte read solves, but it is really slow.
rescue IOError => e
if @gzip
puts "Warning: Bad gzip compression detected. Trying fall back to byte per byte mode."
@str = ""
@file.rewind
@file.each_byte { |byte| @str << byte }
@stream = Stream.new(@str, false)
check_header
else
raise e
end
ensure
@file.close
end

@hdr = parse_basic_header
@extended_header = parse_extended_header

Expand Down Expand Up @@ -217,8 +236,10 @@ def open_file(file)
if File.size(file) > 8
begin
@file = Zlib::GzipReader.new(File.new(file, "rb"))
@gzip = true
rescue Zlib::GzipFile::Error
@file = File.new(file, "rb")
@gzip = false
end
else
@msg << "Error! File is too small to contain DICOM information (#{file})."
Expand Down

3 comments on commit b670ec4

@kastman
Copy link

@kastman kastman commented on b670ec4 Aug 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that file is failing, but I'm glad you found a fix. Any way we can duplicate the error in a good file that could be added to the tests? Let me know if you want me to pull it into master!

@rafamanzo
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Erik,

I've already opened a bug on the Ruby Redmine to see if this issue gets fixed: https://bugs.ruby-lang.org/issues/10101
But, to be honest I don't have big expectations that this might get fixed there, since the ZLib extension doesn't have a maintainer right now.

I'll make a final test later today or tomorrow morning with this patch that I've suggested here and I'll get back to you if it's good for merge.

Thanks!

@rafamanzo
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested it against the file, but besides this fix makes possible to read the header. The image data is completely wrong :(

So It's better to not merge this fix and just raise the current exception instead of giving wrong information :(

I've just cloned the ruby source to see if I understand the issue there...

Please sign in to comment.