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

FANSI_check_enc contains bad call to 'error' that can segfault #48

Closed
msannell opened this issue Jul 19, 2018 · 3 comments
Closed

FANSI_check_enc contains bad call to 'error' that can segfault #48

msannell opened this issue Jul 19, 2018 · 3 comments
Labels
Milestone

Comments

@msannell
Copy link

@msannell msannell commented Jul 19, 2018

FANSI_check_enc contains the code:

error(
  "%d encountered at index %.0f. %s.",
  "Internal Error: unexpected encoding ", type,
  (double) i + 1,
  "Contact maintainer"
);

which has three format specifiers, and four additional arguments.
This should probably be changed to:

error(
  "Internal Error: unexpected encoding %d encountered at index %.0f. %s.",
  type,
  (double) i + 1,
  "Contact maintainer"
);

I discovered this due to a bug in the TERR engine, where getCharCE
produced an incorrect character encoding, leading to a call to this
'error' code, which generated a segfault. This code would probably
never be called in R, but it is worth fixing just in case.

@brodieG brodieG added the bug label Jul 20, 2018
@brodieG brodieG added this to the 0.2.4 milestone Jul 20, 2018
@brodieG
Copy link
Owner

@brodieG brodieG commented Jul 20, 2018

Nice find. Do you have a link to the TERR bug? Based on my use of 'nocov' around that section of code I must have thought at the time it was hard to generate a test case that would hit that point in the code.

@msannell
Copy link
Author

@msannell msannell commented Jul 21, 2018

No, there isn't an publicly visible link to the TERR bug. Part of the bug (now fixed) was that TERR was returning a bad value from getCharCE, which caused it to hit this point in the fansi code.

@brodieG
Copy link
Owner

@brodieG brodieG commented Jul 30, 2018

This is now fixed in the development branch. Thanks for reporting. I'll say though I can't quite figure out how TERR managed to generate borked encoding types given that the mkChar* functions validate the type.

Either way, I should have tested this code rather than assumed it was unreachable.

@brodieG brodieG closed this in 915d0fd Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.