krists / id3tag Goto Github PK

Hey all,

I am working with a very specific mp3 file trying to get all the metadata from it using format parser gem and I got the following error (StringIO throwing on read):

ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/audio_file.rb:53:in `read': negative length -1430667650 given (ArgumentError)

Unfortunately, I cannot share the file within this issue (as I am not the author) and I simply cannot recreate a file with the breaking scenario.

But the problem apparently is the following:

1 - the file has a v2_tag_present so it gets through should_and_could_read_v2_frames? - https://github.com/krists/id3tag/blob/master/lib/id3tag/tag.rb#L118
2 - but when tries gets the v2_tag_body it fails giving the error - here
2.1 - inside the v2_tag_body method, v2_tag_frame_and_paddding_size gives a negative value and stores in bytes_to_read
2.2 - the negative values comes from the v2_extended_header_size being bigger than the v2_tag_header.tag_size
2.2 - then @file.read methods of course fails

I was trying to fix the issue myself but I am fairly new to Ruby and also to working with mp3 files and id3 v2_tags.

But nonetheless, here are some proposals, please let me know your thoughts and I will create a PR:

Proposal 1:

I could simply return BLANK_STRING in v2_tag_body if the bytes_to_read happen to be negative something like this:

    def v2_tag_body
      if v2_tag_size > 0
        @file.seek(v2_tag_frame_and_padding_position)
        bytes_to_read = v2_tag_frame_and_padding_size

       return BLANK_STRING if bytes_to_read < 0

        limit = ID3Tag.configuration.v2_tag_read_limit
        if (limit > 0) && (bytes_to_read > limit)
          bytes_to_read = limit
        end
        @file.read(bytes_to_read) || BLANK_STRING
      end
    end

Proposal 2

I wonder though if the fix would be better to be done somewhere in get_v2_extendend_header_size as it is commented that it could only be either 6 or 10 bytes, but in reality it is giving a much bigger result, but unfortunately I am not sure what value should return in this case or why this number is much bigger than the expected:

    def get_v2_extended_header_size
      if v2_tag_header.extended_header?
        if v2_tag_major_version_number == 3
          # ext. header size for 2.3.0 does not include size bytes.
          # There are only 2 possible sizes - 6 or 10 bytes, which means extended header can take 10 or 14 bytes.
          4 + NumberUtil.convert_string_to_32bit_integer(v2_extended_header_size_bytes)
        else
          SynchsafeInteger.decode(NumberUtil.convert_string_to_32bit_integer(v2_extended_header_size_bytes))
        end
      else
        0
      end
    end

Original error traceback:

Traceback (most recent call last):
        43: from exe/format_parser_inspect:22:in `<main>'
        42: from exe/format_parser_inspect:22:in `map'
        41: from exe/format_parser_inspect:24:in `block in <main>'
        40: from exe/format_parser_inspect:24:in `public_send'
        39: from /****/format_parser/lib/format_parser.rb:109:in `parse_file_at'
        38: from /****/format_parser/lib/format_parser.rb:109:in `open'
        37: from /****/format_parser/lib/format_parser.rb:110:in `block in parse_file_at'
        36: from /****/format_parser/lib/format_parser.rb:178:in `parse'
        35: from /****/format_parser/lib/format_parser.rb:178:in `to_a'
        34: from /****/format_parser/lib/format_parser.rb:178:in `each'
        33: from /****/format_parser/lib/format_parser.rb:178:in `each'
        32: from /****/format_parser/lib/format_parser.rb:178:in `each'
        31: from /****/format_parser/lib/format_parser.rb:178:in `each'
        30: from /****/format_parser/lib/format_parser.rb:174:in `block in parse'
        29: from /****/format_parser/lib/format_parser.rb:206:in `execute_parser_and_capture_expected_exceptions'
        28: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/measurometer-1.3.0/lib/measurometer.rb:48:in `instrument'
        27: from /****/format_parser/lib/format_parser.rb:207:in `block in execute_parser_and_capture_expected_exceptions'
        26: from /****/format_parser/lib/parsers/mp3_parser.rb:103:in `call'
        25: from /****/format_parser/lib/parsers/mp3_parser.rb:318:in `with_id3tag_local_configs'
        24: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag.rb:52:in `local_configuration'
        23: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/configuration.rb:9:in `local_configuration'
        22: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/configuration.rb:32:in `local_configuration'
        21: from /****/format_parser/lib/parsers/mp3_parser.rb:322:in `block in with_id3tag_local_configs'
        20: from /****/format_parser/lib/parsers/mp3_parser.rb:103:in `block in call'
        19: from /****/format_parser/lib/parsers/mp3_parser.rb:302:in `blend_id3_tags_into_hash'
        18: from /****/format_parser/lib/parsers/mp3_parser.rb:302:in `each_with_object'
        17: from /****/format_parser/lib/parsers/mp3_parser.rb:302:in `each'
        16: from /****/format_parser/lib/parsers/mp3_parser.rb:303:in `block in blend_id3_tags_into_hash'
        15: from /****/format_parser/lib/parsers/mp3_parser.rb:50:in `to_h'
        14: from /****/format_parser/lib/parsers/mp3_parser.rb:50:in `each_with_object'
        13: from /****/format_parser/lib/parsers/mp3_parser.rb:50:in `each'
        12: from /****/format_parser/lib/parsers/mp3_parser.rb:52:in `block in to_h'
        11: from /****/format_parser/lib/parsers/mp3_parser.rb:52:in `public_send'
        10: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/tag.rb:19:in `block (2 levels) in <class:Tag>'
         9: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/tag.rb:24:in `content_of_first_frame'
         8: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/tag.rb:40:in `first_frame_by_id'
         7: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/tag.rb:40:in `find'
         6: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/tag.rb:40:in `each'
         5: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/tag.rb:40:in `block in first_frame_by_id'
         4: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/tag.rb:64:in `frame_ids'
         3: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/tag.rb:68:in `frames'
         2: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/tag.rb:73:in `v2_frames'
         1: from /*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/audio_file.rb:59:in `v2_tag_body'
/*/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/id3tag-0.14.1/lib/id3tag/audio_file.rb:59:in `read': negative length -1430667650 given (ArgumentError)

I see Tag#possible_frame_ids_by_name(name) tries v1 tags first and then tries v2.

Wouldn't it be smarter to default to the highest available version and fallback to lower versions?

I'm trying to find a library that would work for flac...

Is seems quite common that id3 writers do not include the BOM for unicode strings. We should provide some kind of fallback functionality for trying to guess a fallback encoding when string encode fails.

Table of BOM guesses per encoding or some kinds of lambda/block? should the lib have some kinds of default fallback?

Example of broken id3 tags:
http://www.bbc.co.uk/podcasts (most or all of the mp3s use 0x01 text encoding but with no BOM. Seems to be little endian)

Hey there, do you have any thoughts on supporting the CHAP frame for chapters? I've been using this JavaScript version which produces an array of chapter objects like so:

{
  id: 'ch0',
  startTime: 0, // in ms
  endTime: 60000, // in ms
  startOffset: 4294967295,
  endTime: 4294967295,
  subFrames: {
    TIT2: {
      id: 'TIT2',
      size: 49,
      description: 'Title/songname/content description',
      data: 'This is a Chapter Title'
    },
    WXXX: {
      id: 'WXXX',
      size: 68,
      description: 'User defined URL link frame',
      data: {
        user_description: 'chapter url',
        data: 'https://www.example.com/foo/this-is-a-user-defined-url/'
      }
    }
  }
}

I'd be happy to help contribute to make this, but I might need a few pointers to get started :)

Thanks!

Here's my current situation:

(byebug) mp3data.frames[3]
<ID3Tag::Frames::V2::GenreFrame TCON: >
(byebug) mp3data.frames[3].raw_content
"\x01\xFF\xFEP\x00o\x00p\x00\x00\x00"
(byebug) mp3data.genre
""

This null character separated genre tag should be "Pop". I've saved this tag using mp3tag, and confirmed that it's ID3v2.3.

Am I doing something wrong here?

I've looked a bit at the ID3v1 and ID3v2 specs, and as far as I can tell, they support a TLEN frame which contains the track length. None of the MP3's I have contain this frame. That said, basically every tagging utility that I've used is able to tell me the length of the tracks I give it.

Is there something obvious that I'm missing that would allow me to obtain the duration? Currently my application only grabs the file headers to get all of the tag data, so ideally I wouldn't have to download the entire file.

How can I use this with a base64 blob???

It wants to open files... But, I want to pass it the blob..

I couldn’t find anything in the code but wanted to make sure.

Currently the library is a reader-only library. Consider writer functionalities?

These methods should accept language to fetch frame for specific language

The size field should be treaded differently in v2.3 and v2.4. v2.3 is not synch safe and excluding size field, v2.4 is synch safe including size field.

Hello @krists
Here I am reading if a file have id3 tags, the file i am loading either have id3 tags or not. If file have id3tags then it is working correctly and if file has no id3 tag then it raised exception.

NoMethodError: undefined method unpack' for nil:NilClass from app/services/uploads/rip_id3.rb:34:in fields'
from app/services/uploads/rip_id3.rb:40:in metadata' from app/services/uploads/rip_id3.rb:11:in call'
from app/models/concerns/service.rb:6:in `call'
from (irb):7

Here is my code.

module Uploads
  class RipId3
    include Service
    attr_reader :upload

    def initialize(upload)
      @upload = upload
    end

    def call
      upload.file_info.deep_merge!(metadata: metadata)
      upload.save!
      upload.processed!
    end

    private

    def temp_file
      @temp_file = begin
        if upload.high_res?
          open(upload.file_ref.url(:mp3))
        else
          open(upload.file_ref.url)
        end
      end
    end

    def id3_tag
        @id3_tag ||= ID3Tag.read(temp_file)
    end

    def fields
       @fields ||= id3_tag.frame_ids
    end

    def metadata
      fields.each_with_object({}) do |field, acc|
        case field
        when :TALB
          acc['album'] = id3_tag.get_frames(field).map(&:content).join(' | ')
        when :TIT2
          acc['title'] = id3_tag.get_frames(field).map(&:content).join(' | ')
        when :TPE1
          acc['artist'] = id3_tag.get_frames(field).map(&:content).join(' | ')
        when :TCON
          acc['genre'] = id3_tag.get_frames(field).map(&:content).join(' | ')
        else
          acc[field] = id3_tag.get_frames(field).map(&:content).join(' | ')
        end
      end
    end
  end
end

How can i use id3tag with activeStorage?

Hello @krists !

I was able to find how to read image object with: tag.get_frame :APIC
Then I got some methods like mime_type and many others: :content, :data, :id, :raw_content, :usable_content

Those that return data/content return me some utf encoded string, at this point i was not able to save it to jpg, can you please give me clue how to forward ?

thank you ! (will dig into lib now)