diff --git a/src/DicomMessage.js b/src/DicomMessage.js index ebde2329..72b89f00 100644 --- a/src/DicomMessage.js +++ b/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; diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index 59f6e9ea..39e4e336 100644 --- a/src/ValueRepresentation.js +++ b/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)); } } diff --git a/test/data.test.js b/test/data.test.js index 8dca1803..10c78ae6 100644 --- a/test/data.test.js +++ b/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"] ], [ diff --git a/test/lossless-read-write.test.js b/test/lossless-read-write.test.js index ad1c43c0..0105c4d7 100644 --- a/test/lossless-read-write.test.js +++ b/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"], }, {