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.

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

Jump to bottom

Validate allows CRLF within a Table_Delimited field #357

Closed
jordanpadams opened this issue Jun 8, 2021 · 4 comments · Fixed by #365
Closed

Validate allows CRLF within a Table_Delimited field #357

jordanpadams opened this issue Jun 8, 2021 · 4 comments · Fixed by #365
Assignees
@qchaupds
Labels
B12.0 bug Something isn't working s.medium
Milestone
10.Lynn.Jennings

Comments

@jordanpadams
Copy link
Member

🐛 Describe the bug

Per 4C.1 of the Standards Reference regarding the PDS DSV:

<CR> and <LF> may be used only in combination as a record delimiter. They may not be used, either separately or in combination, as part of a field value.

We should not allow CRLF within a Table_Delimited field.

📜 To Reproduce

Issue was found per #355 .

🕵️ Expected behavior

Validate should check if there is a CR or LF within a Table_Delimited field, and raise an error if it exists.

📚 Version of Software Used

2.0.6

🩺 Test Data / Additional context

See #355

🦄 Applicable requirements**
@jordanpadams jordanpadams added bug Something isn't working s.medium labels Jun 8, 2021
@jordanpadams jordanpadams added this to the 09.Valerie.Brisco milestone Jun 8, 2021
@qchaupds qchaupds mentioned this issue Jun 25, 2021
Closed
1 task
qchaupds pushed a commit that referenced this issue Jun 29, 2021
Improve validate reporting when trying to read a null row, flag if CR…
0ad5de7 
…/LF is included in field value, modify unallowed name message to be warning instead of error

1. Add test artifact for github356 to src/test/resources.  Cucumber requires a directory for a test to exist eventhough the test uses another input directory
2. Add test artifact for github357 to src/test/resources
3. Add test artifact for github364 to src/test/resources
4. Modify from error to warning for sub_directory.unallowed_name flag in ProblemType.java
5. Add logic to check for CR/LF in field value for Table_Delimited table and add ogic to break out of loop if error is fatal to avoid reporting same error in FieldValueValidator.java
6. Add logic to allow '.XML' to be an extension for a label in LabelValidationRule.java
7. Modify message to include: "with the extension '.xml' or '.XML'" in PDS4Problems.java
8. Add debug statements and logic to report warning if read too many records to TableDataContentValidationRule.java
9. Add BAD_EXTENSION type to support new errors detected to StepDefs.java
10. Add new tests to validate.feature

Refs:

#355 Improve validate reporting when trying to read a null row
#356 validate labels error.sub_directory.unallowed_name as a warning
#357 Validate allows CRLF within a Table_Delimited field
#364 validate does not allow ".XML" as an extension for a label file
@qchaupds qchaupds mentioned this issue Jun 29, 2021
Merged
jordanpadams added a commit that referenced this issue Jun 30, 2021
@jordanpadams
Merge pull request #365 from NASA-PDS/#355_356_357_364
ff600ce 
Improvements and fixes for #355, #356, #357, #364
jordanpadams pushed a commit that referenced this issue Jul 3, 2021
@jordanpadams
Improve validate reporting when trying to read a null row, flag if CR…
4bb827b 
…/LF is included in field value, modify unallowed name message to be warning instead of error

1. Add test artifact for github356 to src/test/resources.  Cucumber requires a directory for a test to exist eventhough the test uses another input directory
2. Add test artifact for github357 to src/test/resources
3. Add test artifact for github364 to src/test/resources
4. Modify from error to warning for sub_directory.unallowed_name flag in ProblemType.java
5. Add logic to check for CR/LF in field value for Table_Delimited table and add ogic to break out of loop if error is fatal to avoid reporting same error in FieldValueValidator.java
6. Add logic to allow '.XML' to be an extension for a label in LabelValidationRule.java
7. Modify message to include: "with the extension '.xml' or '.XML'" in PDS4Problems.java
8. Add debug statements and logic to report warning if read too many records to TableDataContentValidationRule.java
9. Add BAD_EXTENSION type to support new errors detected to StepDefs.java
10. Add new tests to validate.feature

Refs:

#355 Improve validate reporting when trying to read a null row
#356 validate labels error.sub_directory.unallowed_name as a warning
#357 Validate allows CRLF within a Table_Delimited field
#364 validate does not allow ".XML" as an extension for a label file
@jordanpadams jordanpadams mentioned this issue Aug 16, 2021
Closed
14 tasks
@jpl-jengelke
Copy link
Contributor

jpl-jengelke commented Oct 5, 2021 •
edited

@jordanpadams I did not pass testing on this issue. That is because even when CTRL-V is inserted manually or when '\r' or '\n' are entered manually into CSV test data the test never fails. So it appears the CRLFs are not detected.

Moreover, the data appears insufficient to test this particular issue as a fail mode file would make it clear.

Please review. ...
357_execution.txt
test_357-1.txt

@jordanpadams
Copy link
Member Author

@jpl-jengelke this may be a bug. can you open an I&T Bug Report here? https://github.com/NASA-PDS/validate/issues/new/choose

@rchenatjpl
Copy link

Looks like the newer version fixed this. In the attached .csv, record 2 has an extra LF, and record 7 has an extra CR. validate reports (reasonably, not perfectly since the LF looks like a new record):
Begin Content Validation: file:/Users/rchen/Desktop/test/val357.csv
ERROR [error.table.missing_CRLF] table 1, record 2: Record does not end in carriage-return line feed.
ERROR [error.validation.invalid_field_value] table 1, record 2, field 24: Field value cannot contain a carriage return or linefeed for Table_Delimited
ERROR [error.validation.invalid_field_value] table 1, record 6, field 25: Field value cannot contain a carriage return or linefeed for Table_Delimited
ERROR [error.table.missing_CRLF] table 1, record 7: Record does not end in carriage-return line feed.
End Content Validation: file:/Users/rchen/Desktop/test/val357.csv
1 product validation(s) completed
Archive.zip

@jordanpadams
Copy link
Member Author

@rchenatjpl thanks. i agree the error reporting could potentially be improved, but it that require a much more complicated algorithm to move forward through a file, but then when you encounter an issue, go back to see if that is what you meant. if you think that would be helpful, we can add it, but I'm not sure how often this use cases actually happens?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@qchaupds qchaupds

Labels
B12.0 bug Something isn't working s.medium
Projects
None yet
Milestone
10.Lynn.Jennings
Development

Successfully merging a pull request may close this issue.

Improvements and fixes for #355, #356, #357, #364 NASA-PDS/validate
5 participants
@jordanpadams @jpl-jengelke @rchenatjpl @tloubrieu-jpl @qchaupds