Skip to content

Fix NaN incorrectly rejected as invalid value for ASCII_String table fields #1532

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.

Sign up for GitHub

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

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

Fix NaN incorrectly rejected as invalid value for ASCII_String table fields #1532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/main/java/gov/nasa/pds/tools/validate/content/table/FieldValueValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ public class FieldValueValidator {
FieldType.IEEE754LSBSINGLE,
FieldType.IEEE754MSBDOUBLE,
FieldType.IEEE754MSBSINGLE);
// String types that accept any string value, including "NaN" (issue #956)
private static List<FieldType> stringTypes = Arrays.asList(
FieldType.ASCII_STRING);
Comment on lines +67 to +69
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stringTypes is a mutable, non-final static collection. Since it’s used as a constant for type classification, make it static final and preferably an immutable Set (e.g., EnumSet) to avoid accidental modification and to make membership checks clearer.

Copilot uses AI. Check for mistakes.

/** Container to capture messages. */
private ProblemListener listener;
Expand Down Expand Up @@ -607,7 +610,7 @@ private void checkType(String value, FieldType type) throws InvalidTableExceptio

LOG.debug("checkType:value,type:after [{}],[{}]", value, type);

if (SpecialConstantChecker.isInfOrNan(value) && !realTypes.contains(type)) {
if (SpecialConstantChecker.isInfOrNan(value) && !realTypes.contains(type) && !stringTypes.contains(type)) {
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NaN/Inf guard skips only ASCII_STRING, but this method also supports FieldType.UTF8_STRING later in the same chain. As a result, a UTF8_String field containing "NaN" would still throw "NaN" is not allowed. Consider treating all string field types as exempt from the Inf/NaN check (e.g., include UTF8_STRING here, and ideally centralize string-type membership in a single set).

Suggested change
if (SpecialConstantChecker.isInfOrNan(value) && !realTypes.contains(type) && !stringTypes.contains(type)) {
// Treat all string field types (including UTF8_STRING) as exempt from the Inf/NaN check.
final boolean isStringType =
stringTypes.contains(type)
|| FieldType.UTF8_STRING.getXMLType().equals(type.getXMLType());
if (SpecialConstantChecker.isInfOrNan(value) && !realTypes.contains(type) && !isStringType) {

Copilot uses AI. Check for mistakes.
throw new InvalidTableException(value + " is not allowed");
}
if (FieldType.ASCII_INTEGER.getXMLType().equals(type.getXMLType())) {
Expand Down
1 change: 1 addition & 0 deletions src/test/resources/features/4.1.x.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ Feature: 4.1.x
@4.1.x
Examples:
| issueNumber | subtest | datasrc | args | expectation |
| 956 | 1 | "github956" | "--skip-context-validation -t {datasrc}/" | |
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Examples table row is not aligned with the header (missing the expected indentation and leading |). As written, Gherkin will not parse this as a table row for the Scenario Outline examples.

Suggested change
| 956 | 1 | "github956" | "--skip-context-validation -t {datasrc}/" | |
| 956 | 1 | "github956" | "--skip-context-validation -t {datasrc}/" | |

Copilot uses AI. Check for mistakes.
Loading
Loading