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

Misleading error message if "fmt " chunk extension is too large to fit into chunk #39

Closed
jstrait opened this issue Jul 14, 2022 · 1 comment
Assignees
Labels

Comments

@jstrait
Copy link
Owner

jstrait commented Jul 14, 2022

As an example, suppose a "fmt " chunk has a stated size of 30 bytes, and the format code is not 1 (and therefore the chunk should have an extension). Since a "fmt " chunk extension body always starts at byte 18 (0-based), this means there are 12 bytes available for the chunk extension. If the extension has a reported size larger than 12 bytes, then the chunk extension will overflow out of the chunk.

In v1.1.1, this scenario will always cause InvalidFormatError to be raised. However, that happens by an accident of the implementation, and the error message will be misleading: "Not a supported wave file. The format chunk extension is shorter than expected." Although the extension could be shorter than expected, that's not guaranteed, and won't really be the source of the error.

What should happen in this scenario? If some of the bytes that overflow are part of required fields for the chunk extension (such as say, the first 22 bytes of a WAVE_FORMAT_EXTENSIBLE chunk extension), then it seems clear this should result in an error. If the overflowed bytes are just extra inert bytes, then you could argue that no error needs to be raised. However, that could lead to inconsistent behavior between different format codes, because the gem would have to understand the chunk extension format of every one of the many possible format codes in order to implement this in a consistent way. Also, always raising an error is how previous versions of the gem behave. Therefore, it seems preferable to always raise an error if the chunk extension overflows.

This is closely related to #33, because that bug and this one are caused by the same line of code. Despite the overlap, this bug is being opened since the two issues seem distinct from a behavior perspective.

@jstrait jstrait added the Bug label Jul 14, 2022
@jstrait jstrait self-assigned this Jul 14, 2022
@jstrait
Copy link
Owner Author

jstrait commented Dec 30, 2022

This is now fixed in v1.1.2, so closing this issue.

@jstrait jstrait closed this as completed Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant