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

AIFF: Fix handling of padding bytes, safe chunk manipulation #409

Merged
merged 4 commits into from Nov 17, 2019

Conversation

@phw
Copy link
Contributor

phw commented Oct 23, 2019

This refactors the AIFF code to:

  • Fix handling of padding bytes: For chunks with odd data size there must be a padding byte added. This is now consistently handled for all chunk manipulation (resize, insert, delete). Previously this was not handled for all cases, and reading a file which had a ID3 chunk with padding could lead to file corruption since mutagen would write this chunk back with an even size but keep the padding byte.
  • The code can now safely support multiple chunk insert / resize / delete operations while keeping the in-memory representation of all offsets and sizes intact. Previously it was not safe to e.g. both insert and delete chunks in a loaded file, since the in-memory stored offsets for chunks were not updated. This lead to file corruption since data was manipulated at the wrong location

See also the same changes for RIFF files in #408

This allows to safely perform multiple chunk insert / delete / resize operations on loaded IFF files.
@phw phw force-pushed the phw:fix/aiff-padding branch from cd90a5f to ce8cb75 Oct 23, 2019
@phw phw changed the title AIFF: Fix handling of padding bytes, save chunk manipulation AIFF: Fix handling of padding bytes, safe chunk manipulation Oct 23, 2019
@phw phw force-pushed the phw:fix/aiff-padding branch from 880a7a5 to f4d3c40 Oct 23, 2019
@phw

This comment has been minimized.

Copy link
Contributor Author

phw commented Nov 10, 2019

@lazka Any chance to get this fix included in a new release in the not too far future?

This issue actually is rather bad, I'm thinking about disabling AIFF tag writing in Picard for mutagen versions affected by this, but it would be good to have a fixed version available then.

@lazka

This comment has been minimized.

Copy link
Member

lazka commented Nov 13, 2019

I'll look into it this weekend.

@lazka

This comment has been minimized.

Copy link
Member

lazka commented Nov 17, 2019

puh, a bit hard to review with everything mixed together :/ so I'm just going to click merge here

@lazka lazka merged commit 3252c26 into quodlibet:master Nov 17, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
quodlibet.mutagen #20191023.15 succeeded
Details
@lazka

This comment has been minimized.

Copy link
Member

lazka commented Nov 17, 2019

1.43.0 is out

@EvanPurkhiser

This comment has been minimized.

Copy link
Contributor

EvanPurkhiser commented Nov 26, 2019

Thanks for fixing this @phw. That was some of the first low-level-file code I had written 🙂

@phw phw deleted the phw:fix/aiff-padding branch Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.