Skip to content

Conversation

@zagge-cgeo
Copy link

@zagge-cgeo zagge-cgeo commented Dec 12, 2023

When GetAllChildren return nil, nil the following panic happens

panic: runtime error: makeslice: cap out of range

goroutine 1 [running]:
iso9660.(*File).GetChildren(0x54ec60?)

/home/tzachmann/develop/test/go/iso9660/src/iso9660/image_reader.go:254 +0x51

Here is the code that is problematic. When GetAllChildren returns nil, nil make with a size of -2 will be called which results in the above panic.

func (f *File) GetChildren() ([]*File, error) {
     children, err := f.GetAllChildren()
     if err != nil {
         return nil, err
     }

     filteredChildren := make([]*File, 0, len(children)-2)

This patch returns an error in case there are no children or the ReadAt fails.

If in this cases a nil, nil is fine the above code could be changed to

 filteredChildren := make([]*File, 0, len(children))

to fix the problem.

When GetAllChildren return nil, nil the following panic happens

panic: runtime error: makeslice: cap out of range

goroutine 1 [running]:
iso9660.(*File).GetChildren(0x54ec60?)

/home/tzachmann/develop/test/go/iso9660/src/iso9660/image_reader.go:254
+0x51

Here is the code that is problematic. When GetAllChildren returns nil,
nil make with a size of -2 will be called which results in the above
panic.

func (f *File) GetChildren() ([]*File, error) {
     children, err := f.GetAllChildren()
     if err != nil {
         return nil, err
     }

     filteredChildren := make([]*File, 0, len(children)-2)

This patch returns an error in case there are no children or the ReadAt
fails.

If in this cases a nil, nil is fine the above code could be changed to

     filteredChildren := make([]*File, 0, len(children))

to fix the problem.
@codecov
Copy link

codecov bot commented Dec 21, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (08dd38c) 78.94% compared to head (470883c) 78.74%.

❗ Current head 470883c differs from pull request most recent head 51c2226. Consider uploading reports for the commit 51c2226 to get more accurate results

Files Patch % Lines
image_reader.go 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
- Coverage   78.94%   78.74%   -0.20%     
==========================================
  Files           6        6              
  Lines        1211     1214       +3     
==========================================
  Hits          956      956              
- Misses        180      182       +2     
- Partials       75       76       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kdomanski
Copy link
Owner

Great catch! Thanks for the patch.
Do you think you could add a testcase or two? Otherwise I'll take care of it after the holidays.

@zagge-cgeo
Copy link
Author

Do you think you could add a testcase or two?

Unfortunately I have no clue how to test this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants