Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always return a string for StringUtil#cut_at_null_byte #19

Merged
merged 1 commit into from
Jun 13, 2018

Conversation

alex-quiterio
Copy link
Contributor

I came across this strange behavior after reading the following open issue. Here's the stack trace with some insight:

1) FormatParser::MP3Parser decodes and estimates duration for a CBR MP3
     Failure/Error: value = tag.public_send(k)
     
     NoMethodError:
       undefined method `gsub' for nil:NilClass
     # /Users/code/libs/id3tag/lib/id3tag/frames/v2/genre_frame/genre_parser_pre_24.rb:25:in `just_genres'
     # /Users/code/libs/id3tag/lib/id3tag/frames/v2/genre_frame/genre_parser_pre_24.rb:12:in `genres'
     # /Users/code/libs/id3tag/lib/id3tag/frames/v2/genre_frame.rb:23:in `get_genres'
     # /Users/code/libs/id3tag/lib/id3tag/frames/v2/genre_frame.rb:8:in `genres'

The symptom:

The #initialize method of the class ID3Tag::Frames::V2::GenreFrame::GenreParser expects to get a string as a parameter but it can be invoked with nil instead.

Potential problem:

This scenario can happen due to the fact that StringUtil#cut_at_null_byte can return a value different than String. You can find one instance of the issue in D3Tag::Frames::V2::TextFrame

From the point of view of the StringUtil interface, I expect that cut_at_null_byte would cut the string until the first null_byte and return the first piece of the string. That happens in most of the cases but the edge case of "empty string" is still not covered.

Proposed solution:

In order to fix the issue and the symptoms, we just need to make sure #cut_at_null_byte returns always a String.

I've added some specs to the method StringUtil#cut_at_null_byte too.

Let me know if you have any questions or comments. I would be very happy to help :)
Thanks!

@krists
Copy link
Owner

krists commented Jun 13, 2018

@alex-quiterio Thanks for the detailed explanation. I completely agree that StringUtil#cut_at_null_byte should always return a string.

@krists
Copy link
Owner

krists commented Jun 13, 2018

@alex-quiterio I have released new version 0.10.1

@alex-quiterio
Copy link
Contributor Author

@krists thanks a lot 🙇

julik pushed a commit to WeTransfer/format_parser that referenced this pull request Jun 18, 2018
…125)

The guard `rescue nil` can now be safely removed since the original issue
is fixed by krists/id3tag#19

related with: #118
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