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

Provide more informative error message when encountering unexpected n… #689

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

cinquin
Copy link
Contributor

@cinquin cinquin commented Apr 22, 2018

…ewline character in SAM header.

@daviesrob
Copy link
Member

This could print lots of lines if the problem happens at the start of a long header. There's also no way of limiting the output if the header doesn't include a terminating NUL character, which could be a security problem.

It would be best to keep track of the start of the last line so that the dump always begins at a sensible place. Then use memchr() to find out where the current line ends and use an output format of the form "%.*s", len, last_start to restrict the amount of data that gets printed. Or even better might be an extra function that prints a few lines of header with some extra checks to ensure that the output characters are actually printable, as outputting binary nonsense is generally not a good idea.

@jkbonfield
Copy link
Contributor

It's also incorrect to say "unexpected newline" because this error will be triggered on a header that has zero newlines (and also doesn't start with an @).

Not that we can (legally) create such things, but the whole point of this is to spot illegal / corrupted data.

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.

3 participants