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

Adds bad type descriptors and README #57

Merged
merged 3 commits into from
Jan 3, 2020
Merged

Adds bad type descriptors and README #57

merged 3 commits into from
Jan 3, 2020

Conversation

jonwilsdon
Copy link
Contributor

@jonwilsdon jonwilsdon commented Jan 3, 2020

Description of changes:

Adds bad files for each invalid TL pair and a README that describes the files. This came out of the interest shown in #56. Each invalid TL pair is in its own file.

Below are the issues that I noticed when testing it with the various language implementations:

ion-c

  • type_1_length_*.10n (0x1_) gives IERR_INVALID_STATE instead of IERR_INVALID_BINARY
  • type_6_length_0.10n (0x60) passes and produces 0000T instead of failing with IERR_INVALID_BINARY
  • type_14_length_*.10n (0xE_) gives IERR_UNEXPECTED_EOF instead of IERR_INVALID_BINARY
  • type_15_length_*.10n (0xF_) gives IERR_INVALID_STATE instead of IERR_INVALID_BINARY

ion-python

  • type_6_length_1.10n (0x61) fails with TypeError: ord() expected a character, but string of length 0 found instead of amazon.ion.exceptions.IonException: Invalid type octet: 0x61

ion-java

  • type_1_length_14.10n (0x1E) fails with Exception in thread "main" com.amazon.ion.IonException: unrecognized value type encountered: 15 at position 7 instead of Exception in thread "main" com.amazon.ion.IonException: invalid length nibble in boolean value: 14 at position 5
  • type_6_length_0.10n (0x60) passes and produces null.timestamp instead of failing
  • type_6_length_1.10n (0x61) fails with Exception in thread "main" com.amazon.ion.IonException: unexpected EOF in value at position 6 instead of Exception in thread "main" com.amazon.ion.IonException: invalid length nibble in boolean value: 1 at position 5
  • type_14_length_[1,2].10n (0xE_) fails with Exception in thread "main" com.amazon.ion.IonException: unexpected EOF in value at position 6 instead of Exception in thread "main" com.amazon.ion.IonException: invalid length nibble in boolean value: [1,2] at position 5
  • type_14_length_15.10n (0xEF) fails with Exception in thread "main" com.amazon.ion.IonException: invalid binary image at position 5 instead of Exception in thread "main" com.amazon.ion.IonException: invalid length nibble in boolean value: 15 at position 5

ion-js

  • type_1_length_*.10n (0x1_) does not error. Produces undefined when calling reader.value(), except for T1LE (0x1E) which produces true. Note: I also noticed that reader.booleanValue() is able to be called on any value below 0x10 (it returns null on non-boolean values). For values above 0x1F it produces Error: Current value is not a Boolean. (Edit: Binary reader booleanValue can be called on type code 0 elements ion-js#537)
  • type_1_length_0.10n (0x30) produces l [ sign: false ] (which is the same output as 0x31 00) instead of failing.
  • type_4_length_14.10n (0x4E) reads the VarUInt and will make a float if possible. Example: 0x4E 81 errors with Error: Illegal float length: 1 while 0x4E 84 00 00 00 05 produces 7.006492321624085e-45
  • type_6_length_0.10n (0x60) produces null instead of failing.
  • type_14_length_[1,2].10n (0xE_) produces Error: Skipped over end of source. instead of failing because of the type descriptor.
  • type_15_length_*.10n (0xF_) does not error. Calls to reader.value() produce Error: There is no current value. instead of failing.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@tgregg tgregg left a comment

Choose a reason for hiding this comment

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

I'm leaning toward giving these files more accessible names, since they'll often be encountered outside the context of their readme (like in test failures). Something like type1length2.10n probably has less potential to spark initial confusion for someone who isn't familiar with this naming convention.

I can personally attest that it's fairly annoying when I encountering an error in, e.g., good/testfile29.ion even though it doesn't take that long to figure out what the file actually tests.

iontestdata/bad/typecodes/README.md Outdated Show resolved Hide resolved
iontestdata/bad/typecodes/README.md Show resolved Hide resolved
@jonwilsdon
Copy link
Contributor Author

I updated this PR with the requested changes. The file names have been changed and 00 representations added for type 1 values.

Copy link
Contributor

@tgregg tgregg left a comment

Choose a reason for hiding this comment

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

👍

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.

2 participants