Serial-ATA / lofty-rs

Audio metadata library
Apache License 2.0
184 stars 34 forks source link

[DON'T MERGE] id3v2: Reading multiple TCOM tags should not fail #252

Closed uklotzde closed 11 months ago

uklotzde commented 11 months ago

Bug: Trying to read multiple ID3v2 TCOM tags fails with "Bad Frame Length".

I have added a (disabled) regression test.

Serial-ATA commented 11 months ago

So this is really an issue with any text frame with multiple values. I accidentally made it expect each string to be zero terminated. :smile:

https://github.com/Serial-ATA/lofty-rs/blob/51517d3bd10876308332c9a39f122dc625527bc2/src/id3/v2/items/text_information_frame.rs#L40

(Should be false)

uklotzde commented 11 months ago

So this is really an issue with any text frame with multiple values. I accidentally made it expect each string to be zero terminated. 😄

https://github.com/Serial-ATA/lofty-rs/blob/51517d3bd10876308332c9a39f122dc625527bc2/src/id3/v2/items/text_information_frame.rs#L40

(Should be false)

I only tried to reproduce the error with a concrete example at hand and expected that it is related to any multi-valued field.

The (empty) example file does not need to be committed if an example with a multi-valued field could be generated at on the fly. This draft PR could be closed when a fix and test is in place.

Serial-ATA commented 11 months ago

I would've thought there was a test for multi-value frames already. Guess not.

I'll fix this tomorrow if you don't get around to it. I'll probably also get 0.16.0 out around the same time.

uklotzde commented 11 months ago

Replaced by #255.