tfilestream truncate/read bug - MP3 stripping fix#914
Merged
Conversation
whatdoineed2do
pushed a commit
to whatdoineed2do/taglib
that referenced
this pull request
Jul 21, 2019
Closed
4812449 to
520d1e9
Compare
Contributor
|
Thank you for this fix. Out of curiosity if |
Contributor
Author
|
Calling Here's a test prog to demo the issue: |
whatdoineed2do
pushed a commit
to whatdoineed2do/taglib
that referenced
this pull request
Jul 22, 2019
Contributor
|
Since |
…ffered data
to avoid presenting phantom data being 'read'/presented to caller via a fread()
Current bug due to the buffered data can be seen in stripping mp3s of tags
f.strip(ID3v1);
f.strip(APE);
The ID3v1 tag sits at the end of file (strip calls ftruncate()) and the APE
strip performs a readFile() that would return the stream buffered/truncatd data
and reinsert
whatdoineed2do
pushed a commit
to whatdoineed2do/taglib
that referenced
this pull request
Jul 24, 2019
520d1e9 to
f5fb3ee
Compare
Contributor
Author
|
It makes sense. I've made the change and pushed. |
whatdoineed2do
pushed a commit
to whatdoineed2do/taglib
that referenced
this pull request
Jul 25, 2019
whatdoineed2do
pushed a commit
to whatdoineed2do/taglib
that referenced
this pull request
Sep 7, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #913.
The
fread(3C)function potentially buffers the input stream and needsfflushfollowing aftruncate(2)being called; ieThese functions are used by code by
tfilestreamfor some file operations, in particular the stripping of MP3 tags.The missing
fflushcauses a problem with MP3 stripping of APE and ID3v1 tags - these tags reside at the end of the file and in this order if both exist.Stripping functionality for
APEtagsseekto the end of the tag (based onAPELocationandAPEOriginalSize) and performs a blindfread()until it reaches EOF, assuming it to be the ID3v1 tag.Striipping functionality for
ID3v1tag simply performs aftruncate()from the start of the tag position (ID3v1Location).The bug can be observed by:
This means current master executes:
fread()til end of file; the data returned here is potentially buffered (ie reads the ID3v1 tag that was just truncated/stripped)This commit performs a simple
fflush()following theftruncate()to discard any buffered data to ensure no phantom data being returned.The POSIX defintion for
fflush()defines the behavior for input streams, whilst the latest draft C2x standard leaves it undefined.Tested on Linux/Fedora 26.