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

Fix: missing null byte delimeter in PrivateFrame #138

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

0xNF
Copy link
Contributor

@0xNF 0xNF commented Jun 27, 2024

The spec for PrivateFrames says that a null byte should be inserted after the owner_identifier field.

Currently, if one adds a private frame, they will encounter read failures when decoding tags.

This PR adds the null byte to the encoder which lets tags with PrivateFrames be decoded properly.

…yte between owner_identifier and private data fields
@polyfloyd
Copy link
Owner

Cool, thanks for fixing. Please make the CI happy and I'll merge this :)

@0xNF
Copy link
Contributor Author

0xNF commented Jun 28, 2024

I don't know what to do about that style failure.

When I run cargo clippy --all-features -- -Dwarnings on my local device, my output doesn't reflect the same failure:

  Downloaded tokio-macros v2.3.0
  Downloaded quote v1.0.36
  Downloaded bytes v1.6.0
  Downloaded pin-project-lite v0.2.14
  Downloaded proc-macro2 v1.0.86
  Downloaded syn v2.0.68
  Downloaded tokio v1.38.0
  Downloaded 7 crates (1.2 MB) in 0.69s
   Compiling proc-macro2 v1.0.86
   Compiling unicode-ident v1.0.12
    Checking bytes v1.6.0
    Checking pin-project-lite v0.2.14
   Compiling quote v1.0.36
   Compiling syn v2.0.68
   Compiling tokio-macros v2.3.0
    Checking tokio v1.38.0
    Checking id3 v1.13.1 (/mnt/c/Users/0xnf/projects/experiments/rust-id3)
    Finished dev [unoptimized + debuginfo] target(s) in 15.25s

But more than that, I also think it's not correct? The Storage.reader function is implemented by StorageFile, which in turn is called by a bunch of test functions, like plain_write_to_padding and plain_writer_shrink.

This PR doesn't touch the storage objects at all, so I don't want to make a bigger change than necessary. What do you recommend?

@polyfloyd
Copy link
Owner

Oh hmm yeah, I checked locally and that does not seem related. I'll fix it when I have some time :)

@polyfloyd polyfloyd merged commit 7d57c6d into polyfloyd:main Jun 28, 2024
5 of 6 checks passed
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