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

dcm2json: silently ignore -N, --encode-as-number for values outside the range [-(2^53)+1, (2^53)-1] #892

Closed
malaterre opened this issue Feb 4, 2021 · 4 comments
Assignees
Milestone

Comments

@malaterre
Copy link

Describe the bug

dcm2json remains extremely vague about CP-1861 a warning (or code change) should be indicated.

To Reproduce

$ dcm2json -h
[...]
-N,--encode-as-number encode DS, IS, SV and UV values as JSON
numbers; by default DS, IS, SV and UV
values are encoded as JSON strings
[...]

For example:

% dcmtk-3.6.6/dcmdump input.dcm | grep SV 
(0017,100f) SV 1499816949527975237                      #   8, 1 Unknown Tag & Data

leads to:

$ dcm4chee-5.22.6/bin/dcm2json --encode-as-number input.dcm
[...]
   "0017100F" : {
      "Value" : [
         1499816949527975237
      ],
      "vr" : "SV"
   },

Expected behavior

I would have expected two behaviors:

  1. An error stating that number: 1499816949527975237 cannot be represented in JSON
  2. Explicit quote of the number (eventhough option encode-as-number is passed)
@gunterze
Copy link
Member

gunterze commented Feb 4, 2021

JSON (in opposite to Java Script) does not limit number of digits of integer values!

@malaterre
Copy link
Author

Sorry forgot to quote the actual RFC in the original bug report.

This specification allows implementations to set limits on the range
and precision of numbers accepted. Since software that implements
IEEE 754-2008 binary64 (double precision) numbers [IEEE754] is
generally available and widely used, good interoperability can be
achieved by implementations that expect no more precision or range
than these provide, in the sense that implementations will
approximate JSON numbers within the expected precision. A JSON
number such as 1E400 or 3.141592653589793238462643383279 may indicate
potential interoperability problems, since it suggests that the
software that created it expects receiving software to have greater
capabilities for numeric magnitude and precision than is widely
available.

Note that when such software is used, numbers that are integers and
are in the range [-(253)+1, (253)-1] are interoperable in the
sense that implementations will agree exactly on their numeric
values.

@malaterre
Copy link
Author

Same here:

4.3.19 Number value
primitive value corresponding to a double-precision 64-bit binary format IEEE 754 value

NOTEA Number value is a member of the Number type and is a direct representation of a number.

@gunterze
Copy link
Member

gunterze commented Feb 4, 2021

Allowing implementations to set limits is not equals to specifying a limit. And JSON != ECMA-262.
Anyway, I agree, that it causes lesser troubles to silently ignore --encode-as-number for values outside the 64-bit binary format IEEE 754 range.

@gunterze gunterze self-assigned this Feb 4, 2021
@gunterze gunterze added this to the 5.23.1 milestone Feb 4, 2021
@gunterze gunterze changed the title dcm2json: -N,--encode-as-number should be carefully documented dcm2json: silently ignore -N, --encode-as-number for values outside the range [-(2^53)+1, (2^53)-1] Feb 4, 2021
gunterze added a commit that referenced this issue Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants