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

AST library fails to parse floating point values in FITS headers if locale uses commas as a decimal separator #53

Open
confluence opened this issue Mar 4, 2016 · 5 comments

Comments

@confluence
Copy link

I have recently encountered this issue on Ubuntu 14.04 with the locale set to en_ZA.UTF-8, while using software which uses the AST library as a dependency.

This happens because the native sscanf which AST uses to parse the header is locale-aware. I can see that AST provides its own replacement function which it substitutes on some platforms; however, it was not selected on my platform when I installed AST (8.0.7 and several older versions) from the source tarball.

In addition to this, when this error is encountered, AST attempts to print the affected card in an error message, but does not truncate the char array after 80 characters, so it actually attempts to print the entire remainder of the header, which causes a buffer overflow.

Setting the LC_NUMERIC environment variable to e.g. C fixes my problem locally. Is this the correct way of fixing the problem, or should AST use its replacement function on platforms where it detects a locale with a nonstandard separator? My understanding is that FITS files are not locale-aware and should always use the decimal point as a separator, in which case AST's current behaviour seems to be a bug.

@dsberry
Copy link
Member

dsberry commented Mar 4, 2016

Thanks for the detailed bug report. Locale is something we've not thought about before with AST. It seems fairly clear that when reading or writing values through any form of an AST Channel (a FitsChan for instance), the C locale should be used internally within AST. But what is not so clear is how to handle values supplied as string arguments to functions such as astSetC . For these cases, AST doesn't know where the supplied strings came from originally, and so can't tell what locale they use. The same applies to strings returned by astGetC. Maybe it is then the responsibility of the calling code to set the correct locale prior to calling AST.

@confluence
Copy link
Author

I'm involved in the development of the code in question, so I will suggest this to the rest of the team. It's a small change, and I have verified that it works.

@timj
Copy link
Member

timj commented Mar 4, 2016

I think it's clear that for parsing of FITS headers we have to be using the C locale as that is essentially how the FITS standard is defined.

@dsberry
Copy link
Member

dsberry commented Mar 4, 2016

And also when parsing text created by an astChannel (i.e. native AST representation). But the question is what to do about text supplied via other means like astSetC.

@confluence
Copy link
Author

To clarify, our code is using a FitsChan, so it seems like a clear-cut case where the locale should be fixed to C. I have implemented a workaround by setting the locale in our code, at least temporarily.

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

No branches or pull requests

3 participants