fix(lossles-round-trip): Account for padding byte in numeric strings with multiplicity

src/DicomMessage.js

@@ -376,8 +376,9 @@ class DicomMessage {
                 rawValues = rawValue;
                 values = value
                 if (typeof value === "string") {
-                    rawValues = rawValue.split(String.fromCharCode(VM_DELIMITER));
-                    values = value.split(String.fromCharCode(VM_DELIMITER));
+                    const delimiterChar = String.fromCharCode(VM_DELIMITER);
+                    rawValues = vr.dropPadByte(rawValue.split(delimiterChar))
+                    values = vr.dropPadByte(value.split(delimiterChar));
                 }
             } else if (vr.type == "SQ") {
                 rawValues = rawValue;

src/ValueRepresentation.js

@@ -136,6 +136,41 @@ class ValueRepresentation {
         return tag;
     }
 
+    /**
+     * Removes padding byte, if it exists, from the last value in a multiple-value data element.
+     *
+     * This function ensures that data elements with multiple values maintain their integrity for lossless
+     * read/write operations. In cases where the last value of a multi-valued data element is at the maximum allowed length,
+     * an odd-length total can result in a padding byte being added. This padding byte, can cause a length violation
+     * when writing back to the file. To prevent this, we remove the padding byte if it is the only additional character
+     * in the last element. Otherwise, it leaves the values as-is to minimize changes to the original data.
+     *
+     * @param {string[]} values - An array of strings representing the values of a DICOM data element.
+     * @returns {string[]} The modified array, with the padding byte potentially removed from the last value.
+     */
+    dropPadByte(values) {
+        const maxLength = this.maxLength ?? this.maxCharLength;
+        if (!Array.isArray(values) || !maxLength || !this.padByte) {
+            return values;
+        }
+
+        // Only consider multiple-value data elements, as max length issues arise from a delimiter
+        // making the total length odd and necessitating a padding byte.
+        if (values.length > 1) {
+            const padChar = String.fromCharCode(this.padByte);
+            const lastIdx = values.length - 1;
+            const lastValue = values[lastIdx];
+
+            // If the last element is odd and ends with the padding byte trim to avoid potential max length violations during write
+            if (lastValue.length % 2 !== 0 && lastValue.endsWith(padChar)) {
+                values[lastIdx] = lastValue.substring(0, lastValue.length - 1); // Trim the padding byte
+            }
+        }
+
+        return values;
+    }
+
+
     read(stream, length, syntax, readOptions = { forceStoreRaw: false }) {
         if (this.fixed && this.maxLength) {
             if (!length) return this.defaultValue;
@@ -604,7 +639,7 @@ class CodeString extends AsciiStringRepresentation {
 
     readBytes(stream, length) {
         const BACKSLASH = String.fromCharCode(VM_DELIMITER);
-        return stream.readAsciiString(length).split(BACKSLASH);
+        return this.dropPadByte(stream.readAsciiString(length).split(BACKSLASH));
     }
 
     applyFormatting(value) {
@@ -654,29 +689,28 @@ class AttributeTag extends ValueRepresentation {
 class DateValue extends AsciiStringRepresentation {
     constructor(value) {
         super("DA", value);
-        this.maxLength = 18;
+        this.maxLength = 8;
         this.padByte = PADDING_SPACE;
         //this.fixed = true;
         this.defaultValue = "";
     }
 }
 
-class DecimalString extends AsciiStringRepresentation {
-    constructor() {
-        super("DS");
-        this.maxLength = 16;
-        this.padByte = PADDING_SPACE;
-    }
+class NumericStringRepresentation extends AsciiStringRepresentation {
 
     readBytes(stream, length) {
         const BACKSLASH = String.fromCharCode(VM_DELIMITER);
-        const ds = stream.readAsciiString(length);
-        if (ds.indexOf(BACKSLASH) !== -1) {
-            // handle decimal string with multiplicity
-            return ds.split(BACKSLASH);
-        }
+        const numStr = stream.readAsciiString(length);
 
-        return [ds];
+        return this.dropPadByte(numStr.split(BACKSLASH));
+    }
+}
+
+class DecimalString extends NumericStringRepresentation {
+    constructor() {
+        super("DS");
+        this.maxLength = 16;
+        this.padByte = PADDING_SPACE;
     }
 
     applyFormatting(value) {
@@ -694,6 +728,7 @@ class DecimalString extends AsciiStringRepresentation {
 
     convertToString(value) {
         if (value === null) return "";
+        if (typeof value === 'string') return value;
 
         let str = String(value);
         if (str.length > this.maxLength) {
@@ -798,25 +833,13 @@ class FloatingPointDouble extends ValueRepresentation {
     }
 }
 
-class IntegerString extends AsciiStringRepresentation {
+class IntegerString extends NumericStringRepresentation {
     constructor() {
         super("IS");
         this.maxLength = 12;
         this.padByte = PADDING_SPACE;
     }
 
-    readBytes(stream, length) {
-        const BACKSLASH = String.fromCharCode(VM_DELIMITER);
-        const is = stream.readAsciiString(length);
-
-        if (is.indexOf(BACKSLASH) !== -1) {
-            // handle integer string with multiplicity
-            return is.split(BACKSLASH);
-        }
-
-        return [is];
-    }
-
     applyFormatting(value) {
         const formatNumber = (numberStr) => {
             let returnVal = numberStr.trim().replace(/[^0-9.\\\-+e]/gi, "");
@@ -831,6 +854,7 @@ class IntegerString extends AsciiStringRepresentation {
     }
 
     convertToString(value) {
+        if (typeof value === 'string') return value;
         return value === null ? "" : String(value);
     }
 
@@ -1299,7 +1323,7 @@ class UniqueIdentifier extends AsciiStringRepresentation {
         if (result.indexOf(BACKSLASH) === -1) {
             return result
         } else {
-            return result.split(BACKSLASH)
+            return this.dropPadByte(result.split(BACKSLASH));
         }
     }
 

test/data.test.js

@@ -1158,7 +1158,7 @@ describe("test_un_vr", () => {
                 '00041130',
                 'CS',
                 new Uint8Array([0x4F, 0x52, 0x49, 0x47, 0x49, 0x4E, 0x41, 0x4C, 0x20, 0x20, 0x5C, 0x20, 0x50, 0x52, 0x49, 0x4D, 0x41, 0x52, 0x59, 0x20]).buffer,
-                ["ORIGINAL  ", " PRIMARY "],
+                ["ORIGINAL  ", " PRIMARY"],
                 ["ORIGINAL", "PRIMARY"]
             ],
             [

test/lossless-read-write.test.js

@@ -2,11 +2,11 @@ import "regenerator-runtime/runtime.js";
 
 import fs from "fs";
 import dcmjs from "../src/index.js";
-import {deepEqual} from "../src/utilities/deepEqual";
+import { deepEqual } from "../src/utilities/deepEqual";
 
-import {getTestDataset} from "./testUtils";
+import { getTestDataset } from "./testUtils";
 
-const {DicomDict, DicomMessage} = dcmjs.data;
+const { DicomDict, DicomMessage } = dcmjs.data;
 
 
 describe('lossless-read-write', () => {
@@ -128,6 +128,202 @@ describe('lossless-read-write', () => {
         expect(outputDicomDict.dict['00181041'].Value).toEqual([9007199254740992])
     });
 
+    test('test DS with multiplicity > 1 and added space for even padding is read and written correctly', () => {
+        const dataset = {
+            '00200037': {
+                vr: 'DS',
+                Value: [0.99924236548978, -0.0322633220972, -0.0217663285287, 0.02949870928067, 0.99267261121054, -0.1171789789306]
+            }
+        };
+
+        const dicomDict = new DicomDict({});
+        dicomDict.dict = dataset;
+
+        // write and re-read
+        const outputDicomDict = DicomMessage.readFile(dicomDict.write());
+
+        // ensure _rawValue strings have no added trailing spaces
+        const expectedDataset = {
+            '00200037': {
+                vr: 'DS',
+                Value: [0.99924236548978, -0.0322633220972, -0.0217663285287, 0.02949870928067, 0.99267261121054, -0.1171789789306],
+                _rawValue: ["0.99924236548978", "-0.0322633220972", "-0.0217663285287", "0.02949870928067", "0.99267261121054", "-0.1171789789306"]
+            }
+        };
+
+        expect(deepEqual(expectedDataset, outputDicomDict.dict)).toBeTruthy();
+
+        // re-write should succeeed
+        const outputDicomDictPass2 = DicomMessage.readFile(outputDicomDict.write());
+
+        // dataset should still be equal
+        expect(deepEqual(expectedDataset, outputDicomDictPass2.dict)).toBeTruthy();
+    });
+
+    test('test DS with multiplicity > 1 with padding byte on last element within VR max length is losslessly read', () => {
+        const dataset = {
+            '00200037': {
+                vr: 'DS',
+                Value: [0.99924236548978, -0.0322633220972, -0.0217663285287, 0],
+                _rawValue: ["0.99924236548978", "-0.0322633220972", "-0.0217663285287", " +0.00 "]
+            }
+        };
+
+        const dicomDict = new DicomDict({});
+        dicomDict.dict = dataset;
+
+        // write and re-read
+        const outputDicomDict = DicomMessage.readFile(dicomDict.write());
+
+        // ensure _rawValue strings have no added trailing spaces and retain original encoding details for + and spaces
+        const expectedDataset = {
+            '00200037': {
+                vr: 'DS',
+                Value: [0.99924236548978, -0.0322633220972, -0.0217663285287, 0],
+                _rawValue: ["0.99924236548978", "-0.0322633220972", "-0.0217663285287", " +0.00"]
+            }
+        };
+
+        expect(outputDicomDict.dict).toEqual(expectedDataset);
+
+        // re-write should succeeed
+        const outputDicomDictPass2 = DicomMessage.readFile(outputDicomDict.write());
+
+        // dataset should still be equal
+        expect(outputDicomDictPass2.dict).toEqual(expectedDataset);
+    });
+
+    test('test IS with multiplicity > 1 and added space for even padding is read and written correctly', () => {
+        const dataset = {
+            '00081160': {
+                vr: 'IS',
+                Value: [1234, 5678]
+            }
+        };
+
+        const dicomDict = new DicomDict({});
+        dicomDict.dict = dataset;
+
+        // write and re-read
+        const outputDicomDict = DicomMessage.readFile(dicomDict.write());
+
+        // last _rawValue strings does allow trailing space as it does not exceed max length
+        const expectedDataset = {
+            '00081160': {
+                vr: 'IS',
+                Value: [1234, 5678],
+                _rawValue: ["1234", "5678"]
+            }
+        };
+
+        expect(outputDicomDict.dict).toEqual(expectedDataset);
+
+        // re-write should succeeed
+        const outputDicomDictPass2 = DicomMessage.readFile(outputDicomDict.write());
+
+        // dataset should still be equal
+        expect(outputDicomDictPass2.dict).toEqual(expectedDataset);
+    });
+
+    describe('Multiplicity for non-binary String VRs', () => {
+        const maxLengthCases = [
+            {
+                vr: 'AE',
+                Value: ["MAX_LENGTH_CHARS", "MAX_LENGTH_CHARS"],
+                _rawValue: ["MAX_LENGTH_CHARS", "MAX_LENGTH_CHARS"]
+            },
+            {
+                vr: 'AS',
+                Value: ["120D", "045Y"],
+                _rawValue: ["120D", "045Y"]
+            },
+            {
+                vr: 'AT',
+                Value: [0x00207E14, 0x0012839A],
+                _rawValue: [0x00207E14, 0x0012839A],
+            },
+            {
+                vr: 'CS',
+                Value: ["MAX_LENGTH_CHARS", "MAX_LENGTH_CHARS"],
+                _rawValue: ["MAX_LENGTH_CHARS", "MAX_LENGTH_CHARS"]
+            },
+            {
+                vr: 'DA',
+                Value: ["20230826", "20230826"],
+                _rawValue: ["20230826", "20230826"]
+            },
+            {
+                vr: 'DS',
+                Value: [123456789012.345, 123456789012.345],
+                _rawValue: ["123456789012.345", "123456789012.345"]
+            },
+            {
+                vr: 'DT',
+                Value: ["20230826123045.123456+0100", "20230826123045.123456+0100"],
+                _rawValue: ["20230826123045.123456+0100", "20230826123045.123456+0100"]
+            },
+            {
+                vr: 'IS',
+                Value: [123456789012, 123456789012],
+                _rawValue: ["123456789012", "123456789012"]
+            },
+            {
+                vr: 'LO',
+                Value: ["ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOP", "ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOP"],
+                _rawValue: ["ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOP", "ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOP"]
+            },
+            {
+                vr: 'SH',
+                Value: ["ABCDEFGHIJKLMNOP", "ABCDEFGHIJKLMNOP"],
+                _rawValue: ["ABCDEFGHIJKLMNOP", "ABCDEFGHIJKLMNOP"]
+            },
+            {
+                vr: 'UI',
+                Value: ["1.2.840.12345678901234567890123456789012345678901234567890123456", "1.2.840.12345678901234567890123456789012345678901234567890123456"],
+                _rawValue: ["1.2.840.12345678901234567890123456789012345678901234567890123456", "1.2.840.12345678901234567890123456789012345678901234567890123456"]
+            },
+            {
+                vr: 'TM',
+                Value: ["142530.1234567", "142530.1234567"],
+                _rawValue: ["142530.1234567", "142530.1234567"],
+            },
+
+        ];
+
+        test.each(maxLengthCases)(
+            `Test multiple values with VR max length handle pad byte correctly during read and write - $vr`,
+            (dataElement) => {
+                const dataset = {
+                    '00081160': {
+                        vr: dataElement.vr,
+                        Value: dataElement.Value,
+                    }
+                };
+
+                const dicomDict = new DicomDict({});
+                dicomDict.dict = dataset;
+
+                // write and re-read
+                const outputDicomDict = DicomMessage.readFile(dicomDict.write());
+
+                // expect full _rawValue to match following read
+                const expectedDataset = {
+                    '00081160': {
+                        ...dataElement,
+                    }
+                };
+
+                expect(outputDicomDict.dict).toEqual(expectedDataset);
+
+                // re-write should succeed without max length issues
+                const outputDicomDictPass2 = DicomMessage.readFile(outputDicomDict.write());
+
+                // dataset should still be equal
+                expect(expectedDataset).toEqual(outputDicomDictPass2.dict);
+            }
+        );
+    })
+
     describe('Individual VR comparisons', () => {
 
         const unchangedTestCases = [
@@ -148,7 +344,7 @@ describe('lossless-read-write', () => {
             },
             {
                 vr: "CS",
-                _rawValue: ["ORIGINAL  ", " PRIMARY "], // spaces non-significant for interpretation but allowed
+                _rawValue: ["ORIGINAL  ", " PRIMARY"], // spaces non-significant for interpretation but allowed
                 Value: ["ORIGINAL", "PRIMARY"],
             },
             {