From d5232691d67065f03311f86f720369a192ff1b43 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Tue, 10 Mar 2026 18:30:14 -0400 Subject: [PATCH 01/22] fix(BufferStream) Made option optional which looks like it is the expected behavior. Signed-off-by: Luis M. Santos --- src/BufferStream.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BufferStream.js b/src/BufferStream.js index eacdfaef..73a21a5d 100644 --- a/src/BufferStream.js +++ b/src/BufferStream.js @@ -23,7 +23,7 @@ export class BufferStream { constructor(options = null) { this.isLittleEndian = options?.littleEndian || this.isLittleEndian; this.view.defaultSize = options?.defaultSize ?? this.view.defaultSize; - this.clearBuffers = options.clearBuffers || false; + this.clearBuffers = options?.clearBuffers || false; } /** From f613334e77cecbebc75e0e8ac11af7c507868093 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Tue, 10 Mar 2026 18:31:17 -0400 Subject: [PATCH 02/22] feat(vr-tests) began building test harness for key VR behavior prior to refactoring. Signed-off-by: Luis M. Santos --- test/ValueRepresentation.test.js | 182 +++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 test/ValueRepresentation.test.js diff --git a/test/ValueRepresentation.test.js b/test/ValueRepresentation.test.js new file mode 100644 index 00000000..d6f9d9e8 --- /dev/null +++ b/test/ValueRepresentation.test.js @@ -0,0 +1,182 @@ +import fs from "fs"; +import crypto from "crypto"; +import dcmjs from "../src/index.js"; +import { deepEqual } from "../src/utilities/deepEqual"; +import { + DEFLATED_EXPLICIT_LITTLE_ENDIAN, + UNDEFINED_LENGTH, + TagHex +} from "../src/constants/dicom.js"; + +import { getTestDataset } from "./testUtils"; +import { DicomMetaDictionary } from "../src/DicomMetaDictionary"; +import { ValueRepresentation } from "../src/ValueRepresentation"; +import { BufferStream, WriteBufferStream } from "../src/BufferStream"; + +const { DicomDict, DicomMessage } = dcmjs.data; + +describe("vr basic behavior", () => { + describe("storeRaw option", () => { + const dataset = { + "00080008": { + vr: "CS", + Value: ["DERIVED"] + }, + "00082112": { + vr: "SQ", + Value: [ + { + "00081150": { + vr: "UI", + Value: ["1.2.840.10008.5.1.4.1.1.7"] + } + } + ] + }, + "00180050": { + vr: "DS", + Value: [1] + }, + "00181708": { + vr: "IS", + Value: [426] + }, + "00189328": { + vr: "FD", + Value: [30.98] + }, + "0020000D": { + vr: "UI", + Value: [ + "1.3.6.1.4.1.5962.99.1.2280943358.716200484.1363785608958.3.0" + ] + }, + "00400254": { + vr: "LO", + Value: ["DUCTO/GALACTOGRAM 1 DUCT LT"] + }, + "7FE00010": { + vr: "OW", + Value: [new Uint8Array([0x00, 0x00]).buffer] + } + }; + + const rawDataset = { + "00080008": { + vr: "CS", + Value: ["DERIVED"], + _rawValue: ["DERIVED"] + }, + "00082112": { + vr: "SQ", + Value: [ + { + "00081150": { + vr: "UI", + Value: ["1.2.840.10008.5.1.4.1.1.7"], + _rawValue: ["1.2.840.10008.5.1.4.1.1.7"] + } + } + ], + _rawValue: [ + { + "00081150": { + vr: "UI", + Value: ["1.2.840.10008.5.1.4.1.1.7"], + _rawValue: ["1.2.840.10008.5.1.4.1.1.7"] + } + } + ] + }, + "00180050": { + vr: "DS", + Value: [1], + _rawValue: [1] + }, + "00181708": { + vr: "IS", + Value: [426], + _rawValue: [426] + }, + "00189328": { + vr: "FD", + Value: [30.98], + _rawValue: [30.98] + }, + "0020000D": { + vr: "UI", + Value: [ + "1.3.6.1.4.1.5962.99.1.2280943358.716200484.1363785608958.3.0" + ], + _rawValue: [ + "1.3.6.1.4.1.5962.99.1.2280943358.716200484.1363785608958.3.0" + ] + }, + "00400254": { + vr: "LO", + Value: ["DUCTO/GALACTOGRAM 1 DUCT LT"], + _rawValue: ["DUCTO/GALACTOGRAM 1 DUCT LT"] + }, + "7FE00010": { + vr: "OW", + Value: [new Uint8Array([0x00, 0x00]).buffer], + _rawValue: [new Uint8Array([0x00, 0x00]).buffer] + } + }; + + const VRs = [ + { + vr: "US", + funcType: "Uint16", + readFunc: "readUint16", + writeFunc: "writeUint16", + expectedLength: 2, + testValue: 5 + } + ]; + + test("Write DicomDict without _rawValue", async () => { + const dicomDict = new DicomDict({}); + dicomDict.dict = dataset; + + dicomDict.write(); + // No errors here = pass. + }); + + test("Checking write method in VR", async () => { + const fileStream = new WriteBufferStream(4096, true); + let vr = ValueRepresentation.createByTypeString("DS"); + + const result = vr.write(fileStream); + expect(result).toEqual([0]); + }); + + test("Checking write method in VR requesting specific write type", async () => { + VRs.forEach(vrItem => { + const fileStream = new BufferStream(); + let vr = ValueRepresentation.createByTypeString(vrItem.vr); + + const lengths = vr.write(fileStream, vrItem.funcType, vrItem.testValue); + expect(lengths).toEqual([vrItem.expectedLength]); + + + fileStream.reset(); + const result = fileStream[vrItem.readFunc](); + expect(result).toEqual(vrItem.testValue); + }); + }); + + test("Checking writeBytes method in VR requesting specific write type", async () => { + VRs.forEach(vrItem => { + const fileStream = new BufferStream(); + let vr = ValueRepresentation.createByTypeString(vrItem.vr); + + vr.writeBytes(fileStream, vrItem.testValue); + fileStream.reset(); + const result = fileStream[vrItem.readFunc](); + + expect(result).toEqual(vrItem.testValue); + }); + }); + }); +}); From 22dec2f379eba94a63db06637281955cf1b7892e Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Tue, 10 Mar 2026 19:06:15 -0400 Subject: [PATCH 03/22] feat(vr-refactor) documented write and refactored its logic to explicitly accept the value parameter. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 39 +++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index 695a1545..87656c7a 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -249,30 +249,43 @@ class ValueRepresentation { } } - write(stream, type) { - var args = Array.from(arguments); - if (args[2] === null || args[2] === "" || args[2] === undefined) { + /** + * Write array of values into stream with `ValueMultiplicity Delimiter`. + * + * ### Cases & Expected Behavior: + * + * - If `value` is an `Array`: + * - with `valueArgs = ["1.2.840.10008.1.2.1"]` => `singularArgs = Array(1) [1.2.840.10008.1.2.1]` + * - with `valueArgs = ["1.2.840.10008.1.2.1"]` and `multiplicity = true` => `Array(1) [1.2.840.10008.1.2.1]`, `buffer => "1.2.840.10008.1.2.1"` + * - with `valueArgs = ["5", "10"]` and `multiplicity = true` => `Array(1) ["5", "10"]`, `buffer => "5\\10"` + * - Else: + * - **write value** + * + * @param {BufferStream} stream + * @param {string} type + * @returns {*[]} + */ + write(stream, type, value) { + if (value === null || value === "" || value === undefined) { return [stream.writeAsciiString("")]; } else { - var written = [], - valueArgs = args.slice(2), - func = stream["write" + type]; - if (Array.isArray(valueArgs[0])) { - if (valueArgs[0].length < 1) { + let written = []; + const func = stream["write" + type]; + if (Array.isArray(value)) { + if (value.length < 1) { written.push(0); } else { - var self = this; - valueArgs[0].forEach(function (v, k) { + const self = this; + value.forEach(function (v, k) { if (self.allowMultiple() && k > 0) { stream.writeUint8(VM_DELIMITER); } - var singularArgs = [v].concat(valueArgs.slice(1)); - var byteCount = func.apply(stream, singularArgs); + const byteCount = func.apply(stream, [v]); written.push(byteCount); }); } } else { - written.push(func.apply(stream, valueArgs)); + written.push(func.apply(stream, [value])); } return written; } From 589b8b7e048f3d846bfb1ef80e259485abfdbe3c Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 11:25:44 -0400 Subject: [PATCH 04/22] fix(vr-refactor) correct tracking of bytes written by the write method. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index 87656c7a..966155b1 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -276,13 +276,15 @@ class ValueRepresentation { written.push(0); } else { const self = this; + let byteCount = 0; value.forEach(function (v, k) { if (self.allowMultiple() && k > 0) { stream.writeUint8(VM_DELIMITER); + byteCount++; } - const byteCount = func.apply(stream, [v]); - written.push(byteCount); + byteCount += func.apply(stream, [v]); }); + written.push(byteCount); } } else { written.push(func.apply(stream, [value])); From 03750d7fb1378b1d4895f7fb24b527ebeb93814a Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 11:26:17 -0400 Subject: [PATCH 05/22] feat(vr-tests) updated tests to reflect proper writing of value with multiplicity. Signed-off-by: Luis M. Santos --- test/ValueRepresentation.test.js | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/test/ValueRepresentation.test.js b/test/ValueRepresentation.test.js index d6f9d9e8..af33c294 100644 --- a/test/ValueRepresentation.test.js +++ b/test/ValueRepresentation.test.js @@ -131,8 +131,27 @@ describe("vr basic behavior", () => { readFunc: "readUint16", writeFunc: "writeUint16", expectedLength: 2, - testValue: 5 - } + testValue: 5, + expectedValue: 5 + }, + { + vr: "UI", + funcType: "AsciiString", + readFunc: "readAsciiString", + writeFunc: "writeAsciiString", + expectedLength: 19, + testValue: ["1.2.840.10008.1.2.1"], + expectedValue: "1.2.840.10008.1.2.1" + }, + { + vr: "CS", + funcType: "AsciiString", + readFunc: "readAsciiString", + writeFunc: "writeAsciiString", + expectedLength: 3, + testValue: ["5", "5"], + expectedValue: "5\\5" + }, ]; test("Write DicomDict without _rawValue", async () => { @@ -155,14 +174,15 @@ describe("vr basic behavior", () => { VRs.forEach(vrItem => { const fileStream = new BufferStream(); let vr = ValueRepresentation.createByTypeString(vrItem.vr); + vr._allowMultiple = Array.isArray(vrItem.testValue); const lengths = vr.write(fileStream, vrItem.funcType, vrItem.testValue); expect(lengths).toEqual([vrItem.expectedLength]); fileStream.reset(); - const result = fileStream[vrItem.readFunc](); - expect(result).toEqual(vrItem.testValue); + const result = fileStream[vrItem.readFunc](lengths); + expect(result).toEqual(vrItem.expectedValue); }); }); From f8d3e4e13e645456231247bd6caa89bd06824106 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 11:39:47 -0400 Subject: [PATCH 06/22] feat(vr-tests) fixed writeBytes test to accomodate strings and values with multiplicity. Signed-off-by: Luis M. Santos --- test/ValueRepresentation.test.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/ValueRepresentation.test.js b/test/ValueRepresentation.test.js index af33c294..db3d13c4 100644 --- a/test/ValueRepresentation.test.js +++ b/test/ValueRepresentation.test.js @@ -190,12 +190,13 @@ describe("vr basic behavior", () => { VRs.forEach(vrItem => { const fileStream = new BufferStream(); let vr = ValueRepresentation.createByTypeString(vrItem.vr); + vr._allowMultiple = Array.isArray(vrItem.testValue); - vr.writeBytes(fileStream, vrItem.testValue); + const written = vr.writeBytes(fileStream, vrItem.testValue); fileStream.reset(); - const result = fileStream[vrItem.readFunc](); + const result = fileStream[vrItem.readFunc](written); - expect(result).toEqual(vrItem.testValue); + expect(result).toEqual(vrItem.expectedValue); }); }); }); From 7d8d40797b8d1bd4e90841285caeef76b68d4864 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 13:11:08 -0400 Subject: [PATCH 07/22] feat(vr-refactor) Added documentation strings to make it clear the purpose of the current writeBytes method. This method should be renamed to reflect its validation role better. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 84 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index 966155b1..ed6fdac7 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -69,6 +69,74 @@ var binaryVRs = ["FL", "FD", "SL", "SS", "UL", "US", "AT"], length32VRs = ["OB", "OW", "OF", "SQ", "UC", "UR", "UT", "UN", "OD"], singleVRs = ["SQ", "OF", "OW", "OB", "UN"]; +/** + * ## [6.2 Value Representation (VR)](https://dicom.nema.org/medical/dicom/current/output/html/part05.html#sect_6.2) + * The Value Representation of a Data Element describes the data type and format of that Data Element's Value(s). + * [PS3.6](https://dicom.nema.org/medical/dicom/current/output/html/part06.html#PS3.6) lists the VR of each Data Element by + * Data Element Tag. + * + * **Values with VRs constructed of character strings, except in the case of the VR UI, shall be padded with SPACE + * characters (20H, in the Default Character Repertoire) when necessary to achieve even length. Values with a VR + * of UI shall be padded with a single trailing NULL (00H) character when necessary to achieve even length. + * Values with a VR of OB shall be padded with a single trailing NULL byte value (00H) when necessary to achieve + * even length.** + * + * All new VRs defined in future versions of DICOM shall be of the same Data Element Structure as defined in + * [Section 7.1.2](https://dicom.nema.org/medical/dicom/current/output/html/part05.html#sect_7.1.2) + * (i.e., following the format for VRs such as OB, OW, SQ and UN). + * + * ### Note + * + * 1. *Since all new VRs will be defined as specified in + * [Section 7.1.2](https://dicom.nema.org/medical/dicom/current/output/html/part05.html#sect_7.1.2), an + * implementation may choose to ignore VRs not recognized by applying the rules stated in + * [Section 7.1.2](https://dicom.nema.org/medical/dicom/current/output/html/part05.html#sect_7.1.2).* + * + * 2. *When converting a Data Set from an Explicit VR Transfer Syntax to a different Transfer Syntax, an + * implementation may copy Data Elements with unrecognized VRs in the following manner:* + * - *If the endianness of the Transfer Syntaxes is the same, the Value of the Data Element may be copied + * unchanged and if the target Transfer Syntax is Explicit VR, the VR bytes copied unchanged. In practice + * this only applies to Little Endian Transfer Syntaxes, since there was only one Big Endian Transfer Syntax + * defined.* + * - *If the source Transfer Syntax is Little Endian and the target Transfer Syntax is the (retired) + * Big Endian Explicit VR Transfer Syntax, then the Value of the Data Element may be copied unchanged and + * the VR changed to UN, since being unrecognized, whether or not byte swapping is required is unknown. If the + * VR were copied unchanged, the byte order of the Value might or might not be incorrect.* + * - *If the source Transfer Syntax is the (retired) Big Endian Explicit VR Transfer Syntax, then the + * Data Element cannot be copied, because whether or not byte swapping is required is unknown, and there is + * no equivalent of the UN VR to use when the Value is big endian rather than little endian.* + * + * *The issues of whether or not the Data Element may be copied, and what VR to use if copying, do not arise when + * converting a Data Set from Implicit VR Little Endian Transfer Syntax, since the VR would not be present to be + * unrecognized, and if the Data Element VR is not known from a data dictionary, then UN would be used.* + * + * An individual Value, including padding, shall not exceed the Length of Value, except in the case of the last + * Value of a multi-valued field as specified in [Section 6.4](https://dicom.nema.org/medical/dicom/current/output/html/part05.html#sect_6.4). + * + * + * ### Note + * The lengths of Value Representations for which the Character Repertoire can be extended or replaced are + * expressly specified in characters rather than bytes in + * [Table 6.2-1](https://dicom.nema.org/medical/dicom/current/output/html/part05.html#table_6.2-1). This is because + * the mapping from a character to the number of bytes used for that character's encoding may be dependent on the + * character set used. + * + * Escape Sequences used for Code Extension shall not be included in the count of characters. + * + * ### Note + * 1. *For Data Elements that were present in ACR-NEMA 1.0 and 2.0 and that have been retired, the specifications + * of Value Representation and Value Multiplicity provided are recommendations for the purpose of interpreting + * their Values in objects created in accordance with earlier versions of this Standard. These recommendations are + * suggested as most appropriate for a particular Data Element; however, there is no guarantee that historical + * objects will not violate some requirements or specified VR and/or VM.* + * + * 2. *The length of the Value of UC, UR and UT VRs is limited only by the size of the maximum unsigned integer + * representable in a 32 bit VL field minus two, since FFFFFFFFH is reserved and lengths are required to be even.* + * + * 3. *In previous editions of the Standard (see PS3.5-2015a), the TAB character was not listed as permitted for + * the ST, LT and UT VRs. It has been added for the convenience of formatting and the encoding of XML text.* + * + */ class ValueRepresentation { constructor(type) { this.type = type; @@ -293,6 +361,21 @@ class ValueRepresentation { } } + /** + * Method for validating value size and writing padding bytes as needed. + * + * **Values with VRs constructed of character strings, except in the case of the VR UI, shall be padded with SPACE + * characters (20H, in the Default Character Repertoire) when necessary to achieve even length. Values with a VR + * of UI shall be padded with a single trailing NULL (00H) character when necessary to achieve even length. + * Values with a VR of OB shall be padded with a single trailing NULL byte value (00H) when necessary to achieve + * even length.** + * + * @param {BufferStream} stream + * @param {any} value + * @param {number} lengths + * @param {Object} writeOptions + * @returns {number} + */ writeBytes( stream, value, @@ -300,6 +383,7 @@ class ValueRepresentation { writeOptions = { allowInvalidVRLength: false } ) { const { allowInvalidVRLength } = writeOptions; + // Probably should be false by default and then truly confirm sizes. var valid = true, valarr = Array.isArray(value) ? value : [value], total = 0; From 8885bb5766c38a202291e0c971e0476be39998af Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 13:51:35 -0400 Subject: [PATCH 08/22] feat(vr-refactor) created padding byte lookup table to make it simpler and consistent the selection of the padding character at the base VR class level. It's all based on the VR type anyways. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 76 ++++++++++++-------------------------- src/constants/padding.js | 36 ++++++++++++++++++ 2 files changed, 59 insertions(+), 53 deletions(-) create mode 100644 src/constants/padding.js diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index ed6fdac7..a4c31c81 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -1,10 +1,6 @@ import { ReadBufferStream, WriteBufferStream } from "./BufferStream.js"; -import { - PADDING_NULL, - PADDING_SPACE, - PN_COMPONENT_DELIMITER, - VM_DELIMITER -} from "./constants/dicom.js"; +import { PN_COMPONENT_DELIMITER, VM_DELIMITER } from "./constants/dicom.js"; +import { defaultPadding, padBytes } from "./constants/padding"; import { log, validationLog } from "./log.js"; import dicomJson from "./utilities/dicomJson.js"; @@ -146,6 +142,7 @@ class ValueRepresentation { !this._isBinary && singleVRs.indexOf(this.type) == -1; this._isLength32 = length32VRs.indexOf(this.type) != -1; this._storeRaw = true; + this._padByte = ValueRepresentation.selectPadByte(type); } static setDicomMessageClass(dicomMessageClass) { @@ -160,6 +157,10 @@ class ValueRepresentation { Tag = tagClass; } + static selectPadByte(type) { + return padBytes.has(type) ? padBytes.get(type) : defaultPadding; + } + isBinary() { return this._isBinary; } @@ -237,14 +238,14 @@ class ValueRepresentation { */ dropPadByte(values) { const maxLength = this.maxLength ?? this.maxCharLength; - if (!Array.isArray(values) || !maxLength || !this.padByte) { + 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 padChar = String.fromCharCode(this._padByte); const lastIdx = values.length - 1; const lastValue = values[lastIdx]; @@ -295,7 +296,7 @@ class ValueRepresentation { readPaddedAsciiString(stream, length) { if (!length) return ""; - if (stream.peekUint8(length - 1) !== this.padByte) { + if (stream.peekUint8(length - 1) !== this._padByte) { return stream.readAsciiString(length); } else { var val = stream.readAsciiString(length - 1); @@ -309,7 +310,7 @@ class ValueRepresentation { const val = stream.readEncodedString(length); if ( val.length && - val[val.length - 1] !== String.fromCharCode(this.padByte) + val[val.length - 1] !== String.fromCharCode(this._padByte) ) { return val; } else { @@ -383,22 +384,22 @@ class ValueRepresentation { writeOptions = { allowInvalidVRLength: false } ) { const { allowInvalidVRLength } = writeOptions; + const valarr = Array.isArray(value) ? value : [value]; // Probably should be false by default and then truly confirm sizes. - var valid = true, - valarr = Array.isArray(value) ? value : [value], + let valid = true, total = 0; - for (var i = 0; i < valarr.length; i++) { - var checkValue = valarr[i], - checklen = lengths[i], - isString = false, + for (let i = 0; i < valarr.length; i++) { + const checkValue = valarr[i], + checklen = lengths[i]; + let isString = false, displaylen = checklen; if (checkValue === null || allowInvalidVRLength) { valid = true; } else if (this.checkLength) { valid = this.checkLength(checkValue); } else if (this.maxCharLength) { - var check = this.maxCharLength; //, checklen = checkValue.length; + const check = this.maxCharLength; //, checklen = checkValue.length; valid = checkValue.length <= check; displaylen = checkValue.length; isString = true; @@ -407,7 +408,7 @@ class ValueRepresentation { } if (!valid) { - var errmsg = + const errmsg = "Value exceeds max length, vr: " + this.type + ", value: " + @@ -424,9 +425,9 @@ class ValueRepresentation { } //check for odd - var written = total; - if (total & 1) { - stream.writeUint8(this.padByte); + let written = total; + if (total & 1 && this._padByte) { + stream.writeUint8(this._padByte); written++; } return written; @@ -574,7 +575,7 @@ class BinaryRepresentation extends ValueRepresentation { binaryStream.concat(fragStream); if (addPaddingByte) { - binaryStream.writeInt8(this.padByte); + binaryStream.writeInt8(this._padByte); } } } @@ -739,7 +740,6 @@ class ApplicationEntity extends AsciiStringRepresentation { constructor() { super("AE"); this.maxLength = 16; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -755,7 +755,6 @@ class CodeString extends AsciiStringRepresentation { constructor() { super("CS"); this.maxLength = 16; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -780,7 +779,6 @@ class AgeString extends AsciiStringRepresentation { constructor() { super("AS"); this.maxLength = 4; - this.padByte = PADDING_SPACE; this.fixed = true; this.defaultValue = ""; } @@ -791,7 +789,6 @@ class AttributeTag extends ValueRepresentation { super("AT"); this.maxLength = 4; this.valueLength = 4; - this.padByte = PADDING_NULL; this.fixed = true; } @@ -814,7 +811,6 @@ class DateValue extends AsciiStringRepresentation { super("DA", value); this.maxLength = 8; this.rangeMatchingMaxLength = 18; - this.padByte = PADDING_SPACE; //this.fixed = true; this.defaultValue = ""; } @@ -844,7 +840,6 @@ class DecimalString extends NumericStringRepresentation { constructor() { super("DS"); this.maxLength = 16; - this.padByte = PADDING_SPACE; } applyFormatting(value) { @@ -910,7 +905,6 @@ class DateTime extends AsciiStringRepresentation { super("DT"); this.maxLength = 26; this.rangeMatchingMaxLength = 54; - this.padByte = PADDING_SPACE; } checkLength(value) { @@ -929,7 +923,6 @@ class FloatingPointSingle extends ValueRepresentation { constructor() { super("FL"); this.maxLength = 4; - this.padByte = PADDING_NULL; this.fixed = true; this.defaultValue = 0.0; } @@ -956,7 +949,6 @@ class FloatingPointDouble extends ValueRepresentation { constructor() { super("FD"); this.maxLength = 8; - this.padByte = PADDING_NULL; this.fixed = true; this.defaultValue = 0.0; } @@ -983,7 +975,6 @@ class IntegerString extends NumericStringRepresentation { constructor() { super("IS"); this.maxLength = 12; - this.padByte = PADDING_SPACE; } applyFormatting(value) { @@ -1016,7 +1007,6 @@ class LongString extends EncodedStringRepresentation { constructor() { super("LO"); this.maxCharLength = 64; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -1032,7 +1022,6 @@ class LongText extends EncodedStringRepresentation { constructor() { super("LT"); this.maxCharLength = 10240; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -1048,7 +1037,6 @@ class PersonName extends EncodedStringRepresentation { constructor() { super("PN"); this.maxLength = null; - this.padByte = PADDING_SPACE; } static checkComponentLengths(components) { @@ -1140,7 +1128,6 @@ class ShortString extends EncodedStringRepresentation { constructor() { super("SH"); this.maxCharLength = 16; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -1156,7 +1143,6 @@ class SignedLong extends ValueRepresentation { constructor() { super("SL"); this.maxLength = 4; - this.padByte = PADDING_NULL; this.fixed = true; this.defaultValue = 0; } @@ -1179,7 +1165,6 @@ class SequenceOfItems extends ValueRepresentation { constructor() { super("SQ"); this.maxLength = null; - this.padByte = PADDING_NULL; this.noMultiple = true; this._storeRaw = false; } @@ -1316,7 +1301,6 @@ class SignedShort extends ValueRepresentation { super("SS"); this.maxLength = 2; this.valueLength = 2; - this.padByte = PADDING_NULL; this.fixed = true; this.defaultValue = 0; } @@ -1339,7 +1323,6 @@ class ShortText extends EncodedStringRepresentation { constructor() { super("ST"); this.maxCharLength = 1024; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -1356,7 +1339,6 @@ class TimeValue extends AsciiStringRepresentation { super("TM"); this.maxLength = 16; this.rangeMatchingMaxLength = 28; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -1384,7 +1366,6 @@ class UnlimitedCharacters extends EncodedStringRepresentation { super("UC"); this.maxLength = null; this.multi = true; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -1400,7 +1381,6 @@ class UnlimitedText extends EncodedStringRepresentation { constructor() { super("UT"); this.maxLength = null; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -1416,7 +1396,6 @@ class UnsignedShort extends ValueRepresentation { constructor() { super("US"); this.maxLength = 2; - this.padByte = PADDING_NULL; this.fixed = true; this.defaultValue = 0; } @@ -1439,7 +1418,6 @@ class UnsignedLong extends ValueRepresentation { constructor() { super("UL"); this.maxLength = 4; - this.padByte = PADDING_NULL; this.fixed = true; this.defaultValue = 0; } @@ -1462,7 +1440,6 @@ class UniqueIdentifier extends AsciiStringRepresentation { constructor() { super("UI"); this.maxLength = 64; - this.padByte = PADDING_NULL; } readBytes(stream, length) { @@ -1502,7 +1479,6 @@ class UniversalResource extends AsciiStringRepresentation { constructor() { super("UR"); this.maxLength = null; - this.padByte = PADDING_SPACE; } readBytes(stream, length) { @@ -1514,7 +1490,6 @@ class UnknownValue extends BinaryRepresentation { constructor() { super("UN"); this.maxLength = null; - this.padByte = PADDING_NULL; this.noMultiple = true; } } @@ -1523,7 +1498,6 @@ class ParsedUnknownValue extends BinaryRepresentation { constructor(vr) { super(vr); this.maxLength = null; - this.padByte = 0; this.noMultiple = true; this._isBinary = true; this._allowMultiple = false; @@ -1563,7 +1537,6 @@ class OtherWordString extends BinaryRepresentation { constructor() { super("OW"); this.maxLength = null; - this.padByte = PADDING_NULL; this.noMultiple = true; } } @@ -1572,7 +1545,6 @@ class OtherByteString extends BinaryRepresentation { constructor() { super("OB"); this.maxLength = null; - this.padByte = PADDING_NULL; this.noMultiple = true; } } @@ -1581,7 +1553,6 @@ class OtherDoubleString extends BinaryRepresentation { constructor() { super("OD"); this.maxLength = null; - this.padByte = PADDING_NULL; this.noMultiple = true; } } @@ -1590,7 +1561,6 @@ class OtherFloatString extends BinaryRepresentation { constructor() { super("OF"); this.maxLength = null; - this.padByte = PADDING_NULL; this.noMultiple = true; } } diff --git a/src/constants/padding.js b/src/constants/padding.js new file mode 100644 index 00000000..91fce149 --- /dev/null +++ b/src/constants/padding.js @@ -0,0 +1,36 @@ +import { PADDING_NULL, PADDING_SPACE } from "./dicom"; + +export const padBytes = new Map([ + ["AT", PADDING_NULL], + ["FL", PADDING_NULL], + ["FD", PADDING_NULL], + ["SL", PADDING_NULL], + ["SQ", PADDING_NULL], + ["SS", PADDING_NULL], + ["US", PADDING_NULL], + ["UL", PADDING_NULL], + ["UI", PADDING_NULL], + ["UN", PADDING_NULL], + ["OW", PADDING_NULL], + ["OB", PADDING_NULL], + ["OD", PADDING_NULL], + ["OF", PADDING_NULL], + ["AE", PADDING_SPACE], + ["CS", PADDING_SPACE], + ["AS", PADDING_SPACE], + ["DA", PADDING_SPACE], + ["DS", PADDING_SPACE], + ["DT", PADDING_SPACE], + ["IS", PADDING_SPACE], + ["LO", PADDING_SPACE], + ["LT", PADDING_SPACE], + ["PN", PADDING_SPACE], + ["SH", PADDING_SPACE], + ["ST", PADDING_SPACE], + ["TM", PADDING_SPACE], + ["UC", PADDING_SPACE], + ["UT", PADDING_SPACE], + ["UR", PADDING_SPACE] +]); + +export const defaultPadding = null; From 4f90653037f125a8d9c8e4c3d4e118a10b4293ce Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 15:12:25 -0400 Subject: [PATCH 09/22] feat(vr-refactor) refactored write and writeBytes to simplify the logic behind generating multiplicity write length and its validity/verification. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 75 +++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 42 deletions(-) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index a4c31c81..b181aab1 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -332,31 +332,30 @@ class ValueRepresentation { * * @param {BufferStream} stream * @param {string} type + * @param {any | any[]} value * @returns {*[]} */ write(stream, type, value) { if (value === null || value === "" || value === undefined) { return [stream.writeAsciiString("")]; } else { - let written = []; + let written = 0; const func = stream["write" + type]; if (Array.isArray(value)) { if (value.length < 1) { written.push(0); } else { const self = this; - let byteCount = 0; value.forEach(function (v, k) { if (self.allowMultiple() && k > 0) { stream.writeUint8(VM_DELIMITER); - byteCount++; + written++; } - byteCount += func.apply(stream, [v]); + written += func.apply(stream, [v]); }); - written.push(byteCount); } } else { - written.push(func.apply(stream, [value])); + written += func.apply(stream, [value]); } return written; } @@ -373,60 +372,52 @@ class ValueRepresentation { * * @param {BufferStream} stream * @param {any} value - * @param {number} lengths + * @param {number} length * @param {Object} writeOptions * @returns {number} */ writeBytes( stream, value, - lengths, + length, writeOptions = { allowInvalidVRLength: false } ) { const { allowInvalidVRLength } = writeOptions; - const valarr = Array.isArray(value) ? value : [value]; // Probably should be false by default and then truly confirm sizes. let valid = true, total = 0; - for (let i = 0; i < valarr.length; i++) { - const checkValue = valarr[i], - checklen = lengths[i]; - let isString = false, - displaylen = checklen; - if (checkValue === null || allowInvalidVRLength) { - valid = true; - } else if (this.checkLength) { - valid = this.checkLength(checkValue); - } else if (this.maxCharLength) { - const check = this.maxCharLength; //, checklen = checkValue.length; - valid = checkValue.length <= check; - displaylen = checkValue.length; - isString = true; - } else if (this.maxLength) { - valid = checklen <= this.maxLength; - } - - if (!valid) { - const errmsg = - "Value exceeds max length, vr: " + - this.type + - ", value: " + - checkValue + - ", length: " + - displaylen; - if (isString) log.info(errmsg); - else throw new Error(errmsg); - } - total += checklen; + let isString = false, + displaylen = length; + if (value === null || allowInvalidVRLength) { + valid = true; + } else if (this.checkLength) { + valid = this.checkLength(value); + } else if (this.maxCharLength) { + const check = this.maxCharLength; //, checklen = checkValue.length; + valid = checkValue.length <= check; + displaylen = checkValue.length; + isString = true; + } else if (this.maxLength) { + valid = displaylen <= this.maxLength; } - if (this.allowMultiple()) { - total += valarr.length ? valarr.length - 1 : 0; + + if (!valid) { + const errmsg = + "Value exceeds max length, vr: " + + this.type + + ", value: " + + value + + ", length: " + + displaylen; + if (isString) log.info(errmsg); + else throw new Error(errmsg); } + total += displaylen; //check for odd let written = total; - if (total & 1 && this._padByte) { + if (total & 1 && this._padByte !== null) { stream.writeUint8(this._padByte); written++; } From 61c1716b5b304b7ba34b8a82c7ce205cf1046031 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 15:15:56 -0400 Subject: [PATCH 10/22] feat(vr-refactor) refactored usage of stream read string methods to use the VR's padded versions since they handle dropping off the padding byte from the returned output thus making reads consistent with the writes. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index b181aab1..301ca27b 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -291,7 +291,7 @@ class ValueRepresentation { } readBytes(stream, length) { - return stream.readAsciiString(length); + return super.readPaddedAsciiString(stream, length); } readPaddedAsciiString(stream, length) { @@ -465,7 +465,7 @@ class AsciiStringRepresentation extends ValueRepresentation { } readBytes(stream, length) { - return stream.readAsciiString(length); + return super.readPaddedAsciiString(stream, length); } writeBytes(stream, value, writeOptions) { @@ -481,7 +481,7 @@ class EncodedStringRepresentation extends ValueRepresentation { } readBytes(stream, length) { - return stream.readEncodedString(length); + return super.readPaddedEncodedString(stream, length); } writeBytes(stream, value, writeOptions) { @@ -734,7 +734,7 @@ class ApplicationEntity extends AsciiStringRepresentation { } readBytes(stream, length) { - return stream.readAsciiString(length); + return super.readPaddedAsciiString(stream, length); } applyFormatting(value) { @@ -751,7 +751,7 @@ class CodeString extends AsciiStringRepresentation { readBytes(stream, length) { const BACKSLASH = String.fromCharCode(VM_DELIMITER); return this.dropPadByte( - stream.readAsciiString(length).split(BACKSLASH) + super.readPaddedAsciiString(stream, length).split(BACKSLASH) ); } @@ -821,7 +821,7 @@ class DateValue extends AsciiStringRepresentation { class NumericStringRepresentation extends AsciiStringRepresentation { readBytes(stream, length) { const BACKSLASH = String.fromCharCode(VM_DELIMITER); - const numStr = stream.readAsciiString(length); + const numStr = super.readPaddedAsciiString(stream, length); return this.dropPadByte(numStr.split(BACKSLASH)); } @@ -1001,7 +1001,7 @@ class LongString extends EncodedStringRepresentation { } readBytes(stream, length) { - return stream.readEncodedString(length); + return super.readPaddedEncodedString(stream, length); } applyFormatting(value) { @@ -1016,7 +1016,7 @@ class LongText extends EncodedStringRepresentation { } readBytes(stream, length) { - return stream.readEncodedString(length); + return super.readPaddedEncodedString(stream, length); } applyFormatting(value) { @@ -1122,7 +1122,7 @@ class ShortString extends EncodedStringRepresentation { } readBytes(stream, length) { - return stream.readEncodedString(length); + return super.readPaddedEncodedString(stream, length); } applyFormatting(value) { @@ -1317,7 +1317,7 @@ class ShortText extends EncodedStringRepresentation { } readBytes(stream, length) { - return stream.readEncodedString(length); + return super.readPaddedEncodedString(stream, length); } applyFormatting(value) { @@ -1333,7 +1333,7 @@ class TimeValue extends AsciiStringRepresentation { } readBytes(stream, length) { - return stream.readAsciiString(length); + return super.readPaddedAsciiString(stream, length); } applyFormatting(value) { @@ -1360,7 +1360,7 @@ class UnlimitedCharacters extends EncodedStringRepresentation { } readBytes(stream, length) { - return stream.readEncodedString(length); + return super.readPaddedEncodedString(stream, length); } applyFormatting(value) { @@ -1375,7 +1375,7 @@ class UnlimitedText extends EncodedStringRepresentation { } readBytes(stream, length) { - return stream.readEncodedString(length); + return super.readPaddedEncodedString(stream, length); } applyFormatting(value) { @@ -1473,7 +1473,7 @@ class UniversalResource extends AsciiStringRepresentation { } readBytes(stream, length) { - return stream.readAsciiString(length); + return super.readPaddedAsciiString(stream, length); } } From f1cac6a151b1d6e8514defd6d1168685a84a3db2 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 15:18:25 -0400 Subject: [PATCH 11/22] feat(vr-tests) updated tests to reflect the difference of output from the specialized VR's read/write methods and direct reads from the stream. The latter is present to check that the generated raw value is correct. The former is there to validate that f(x) => y && g(y) => x in terms of the read/write API. Signed-off-by: Luis M. Santos --- test/ValueRepresentation.test.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/test/ValueRepresentation.test.js b/test/ValueRepresentation.test.js index db3d13c4..6fae551b 100644 --- a/test/ValueRepresentation.test.js +++ b/test/ValueRepresentation.test.js @@ -129,28 +129,28 @@ describe("vr basic behavior", () => { vr: "US", funcType: "Uint16", readFunc: "readUint16", - writeFunc: "writeUint16", expectedLength: 2, testValue: 5, - expectedValue: 5 + expectedValue: 5, + expectedRawValue: 5 }, { vr: "UI", funcType: "AsciiString", readFunc: "readAsciiString", - writeFunc: "writeAsciiString", expectedLength: 19, testValue: ["1.2.840.10008.1.2.1"], + expectedRawValue: "1.2.840.10008.1.2.1", expectedValue: "1.2.840.10008.1.2.1" }, { vr: "CS", funcType: "AsciiString", readFunc: "readAsciiString", - writeFunc: "writeAsciiString", expectedLength: 3, testValue: ["5", "5"], - expectedValue: "5\\5" + expectedRawValue: "5\\5", + expectedValue: ["5", "5"] }, ]; @@ -170,19 +170,20 @@ describe("vr basic behavior", () => { expect(result).toEqual([0]); }); - test("Checking write method in VR requesting specific write type", async () => { + test("Checking write method in VR requesting specific write type and confirmed with raw value", async () => { VRs.forEach(vrItem => { const fileStream = new BufferStream(); let vr = ValueRepresentation.createByTypeString(vrItem.vr); vr._allowMultiple = Array.isArray(vrItem.testValue); const lengths = vr.write(fileStream, vrItem.funcType, vrItem.testValue); - expect(lengths).toEqual([vrItem.expectedLength]); + expect(lengths).toEqual(vrItem.expectedLength); fileStream.reset(); + // Checking at the stream level because it returns a more raw result. const result = fileStream[vrItem.readFunc](lengths); - expect(result).toEqual(vrItem.expectedValue); + expect(result).toEqual(vrItem.expectedRawValue); }); }); @@ -194,7 +195,7 @@ describe("vr basic behavior", () => { const written = vr.writeBytes(fileStream, vrItem.testValue); fileStream.reset(); - const result = fileStream[vrItem.readFunc](written); + const result = vr.readBytes(fileStream, written); expect(result).toEqual(vrItem.expectedValue); }); From 6ccdd548b7191e398727a22d6679599fff68807a Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 15:51:54 -0400 Subject: [PATCH 12/22] feat(vr-refactor) fixing minor defect in how length is handled based on prior change. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index 301ca27b..a79c7a18 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -384,22 +384,17 @@ class ValueRepresentation { ) { const { allowInvalidVRLength } = writeOptions; // Probably should be false by default and then truly confirm sizes. - let valid = true, + let valid = value === null || allowInvalidVRLength, total = 0; - let isString = false, - displaylen = length; - if (value === null || allowInvalidVRLength) { - valid = true; - } else if (this.checkLength) { + let isString = false; + if (this.checkLength) { valid = this.checkLength(value); } else if (this.maxCharLength) { - const check = this.maxCharLength; //, checklen = checkValue.length; - valid = checkValue.length <= check; - displaylen = checkValue.length; + valid = length <= this.maxCharLength; isString = true; } else if (this.maxLength) { - valid = displaylen <= this.maxLength; + valid = length <= this.maxLength; } if (!valid) { @@ -409,11 +404,11 @@ class ValueRepresentation { ", value: " + value + ", length: " + - displaylen; + length; if (isString) log.info(errmsg); else throw new Error(errmsg); } - total += displaylen; + total += length; //check for odd let written = total; From 6b3b6b95a4e20a49c1c0d486148dbd123fa79c6d Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 15:52:40 -0400 Subject: [PATCH 13/22] feat(vr-tests) updated test to account for LongString VR type. Signed-off-by: Luis M. Santos --- test/ValueRepresentation.test.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/ValueRepresentation.test.js b/test/ValueRepresentation.test.js index 6fae551b..6fee9685 100644 --- a/test/ValueRepresentation.test.js +++ b/test/ValueRepresentation.test.js @@ -152,6 +152,15 @@ describe("vr basic behavior", () => { expectedRawValue: "5\\5", expectedValue: ["5", "5"] }, + { + vr: "LO", + funcType: "AsciiString", + readFunc: "readEncodedString", + expectedLength: 12, + testValue: "I ♥ my wife!", + expectedRawValue: "I e my wife!", + expectedValue: "I ♥ my wife!" + }, ]; test("Write DicomDict without _rawValue", async () => { @@ -195,7 +204,7 @@ describe("vr basic behavior", () => { const written = vr.writeBytes(fileStream, vrItem.testValue); fileStream.reset(); - const result = vr.readBytes(fileStream, written); + let result = vr.readBytes(fileStream, written); expect(result).toEqual(vrItem.expectedValue); }); From 2cf1fc39862eb3aef7b156ad9d75b15ff9decaaf Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 16:01:37 -0400 Subject: [PATCH 14/22] feat(vr-tests) updated test to account for SignedInteger VR type. Signed-off-by: Luis M. Santos --- test/ValueRepresentation.test.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/ValueRepresentation.test.js b/test/ValueRepresentation.test.js index 6fee9685..77d47d20 100644 --- a/test/ValueRepresentation.test.js +++ b/test/ValueRepresentation.test.js @@ -154,13 +154,22 @@ describe("vr basic behavior", () => { }, { vr: "LO", - funcType: "AsciiString", + funcType: "UTF8String", readFunc: "readEncodedString", - expectedLength: 12, + expectedLength: 14, testValue: "I ♥ my wife!", - expectedRawValue: "I e my wife!", + expectedRawValue: "I ♥ my wife!", expectedValue: "I ♥ my wife!" }, + { + vr: "SL", + funcType: "Int32", + readFunc: "readInt32", + expectedLength: 4, + testValue: -5, + expectedRawValue: -5, + expectedValue: -5 + }, ]; test("Write DicomDict without _rawValue", async () => { From 478327445123cb73d0cb923b11f4b75930e2bf81 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 16:04:45 -0400 Subject: [PATCH 15/22] feat(vr-refactor) making sue we pass a single value as length to the new writeBytes. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index a79c7a18..b5806566 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -585,7 +585,7 @@ class BinaryRepresentation extends ValueRepresentation { return super.writeBytes( stream, binaryData, - [binaryStream.size], + binaryStream.size, writeOptions ); } @@ -1278,7 +1278,7 @@ class SequenceOfItems extends ValueRepresentation { super.write(stream, "Uint32", 0x00000000); written += 8; - return super.writeBytes(stream, value, [written], writeOptions); + return super.writeBytes(stream, value, written, writeOptions); } } From 6a8393f71780508f2c525ac036e17a05fa7fc7f0 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 16:20:56 -0400 Subject: [PATCH 16/22] feat(vr-refactor) refactored write again to make sure only one length value is emitted. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index b5806566..c0676a34 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -336,15 +336,14 @@ class ValueRepresentation { * @returns {*[]} */ write(stream, type, value) { + const func = stream["write" + type]; + let written = 0; + if (value === null || value === "" || value === undefined) { - return [stream.writeAsciiString("")]; + return stream.writeAsciiString(""); } else { - let written = 0; - const func = stream["write" + type]; if (Array.isArray(value)) { - if (value.length < 1) { - written.push(0); - } else { + if (value.length >= 1) { const self = this; value.forEach(function (v, k) { if (self.allowMultiple() && k > 0) { From 88ff1dab6f367495bcb5efb9a82ac01917b078e0 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 17:02:33 -0400 Subject: [PATCH 17/22] feat(vr-refactor) changed this.endOffset check to this.size since calling reset resets this value even if the stream contents and size remain. As a result, an error is thrown inappropriately. Signed-off-by: Luis M. Santos --- src/BufferStream.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BufferStream.js b/src/BufferStream.js index 73a21a5d..11eae5f7 100644 --- a/src/BufferStream.js +++ b/src/BufferStream.js @@ -210,7 +210,7 @@ export class BufferStream { } readUint8Array(length) { - if (this.offset + length > this.endOffset) { + if (this.offset + length > this.size) { throw new Error( `Stream has insufficient data: requested ${length} bytes at offset ${ this.offset From c15fb8dc675e1dd35a534ab613392bb19d22b3d7 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 17:03:54 -0400 Subject: [PATCH 18/22] feat(vr-refactor) making sure the max length of a binary buffer is passed down to writeBytes and properly pass validity for unbounded buffers. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index c0676a34..f35457d9 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -581,6 +581,8 @@ class BinaryRepresentation extends ValueRepresentation { var binaryData = value[0]; binaryStream = new ReadBufferStream(binaryData); stream.concat(binaryStream); + // Make sure we can pass validation of the binary blob since these can be unbounded. + this.maxLength = this.maxLength ?? binaryData.byteLength; return super.writeBytes( stream, binaryData, From e9f31467ecac9b4692c25eebea450f86e0f4673f Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 17:05:24 -0400 Subject: [PATCH 19/22] feat(vr-tests) Added binary OB vr test to make sure we test binary types. Unfortunately, these types write directly via a writeBytes override and thus I make sure here that we use that method when testing binary buffers. Signed-off-by: Luis M. Santos --- test/ValueRepresentation.test.js | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/test/ValueRepresentation.test.js b/test/ValueRepresentation.test.js index 77d47d20..1154024c 100644 --- a/test/ValueRepresentation.test.js +++ b/test/ValueRepresentation.test.js @@ -17,6 +17,8 @@ const { DicomDict, DicomMessage } = dcmjs.data; describe("vr basic behavior", () => { describe("storeRaw option", () => { + const binaryVR = new Set(["OB"]); + const binaryBuffer = new Uint8Array([5,6,7,54,3,255,6,4,63,23]); const dataset = { "00080008": { vr: "CS", @@ -170,6 +172,15 @@ describe("vr basic behavior", () => { expectedRawValue: -5, expectedValue: -5 }, + { + vr: "OB", + funcType: "Uint8Repeat", + readFunc: "readUint8Array", + expectedLength: 10, + testValue: [binaryBuffer.buffer], + expectedRawValue: binaryBuffer, + expectedValue: binaryBuffer + }, ]; test("Write DicomDict without _rawValue", async () => { @@ -192,15 +203,20 @@ describe("vr basic behavior", () => { VRs.forEach(vrItem => { const fileStream = new BufferStream(); let vr = ValueRepresentation.createByTypeString(vrItem.vr); - vr._allowMultiple = Array.isArray(vrItem.testValue); + vr._allowMultiple = Array.isArray(vrItem.testValue) && !binaryVR.has(vrItem.vr); - const lengths = vr.write(fileStream, vrItem.funcType, vrItem.testValue); - expect(lengths).toEqual(vrItem.expectedLength); + let written = -1; + if (binaryVR.has(vrItem.vr)) { + written = vr.writeBytes(fileStream, vrItem.testValue); + } else { + written = vr.write(fileStream, vrItem.funcType, vrItem.testValue); + } + expect(written).toEqual(vrItem.expectedLength); fileStream.reset(); // Checking at the stream level because it returns a more raw result. - const result = fileStream[vrItem.readFunc](lengths); + const result = fileStream[vrItem.readFunc](written); expect(result).toEqual(vrItem.expectedRawValue); }); }); @@ -209,7 +225,7 @@ describe("vr basic behavior", () => { VRs.forEach(vrItem => { const fileStream = new BufferStream(); let vr = ValueRepresentation.createByTypeString(vrItem.vr); - vr._allowMultiple = Array.isArray(vrItem.testValue); + vr._allowMultiple = Array.isArray(vrItem.testValue) && !binaryVR.has(vrItem.vr); const written = vr.writeBytes(fileStream, vrItem.testValue); fileStream.reset(); From 2fd41323dbf7b860ba84aa6d054753e5b0dff0c3 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 17:32:54 -0400 Subject: [PATCH 20/22] feat(vr-refactors) improved logic for determining what the max buffer size should be for a binary vr. The new way addresses an issue with the dicom dict instance ending up with the improper maxLength on construction in data-tests. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index f35457d9..0387891f 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -582,7 +582,10 @@ class BinaryRepresentation extends ValueRepresentation { binaryStream = new ReadBufferStream(binaryData); stream.concat(binaryStream); // Make sure we can pass validation of the binary blob since these can be unbounded. - this.maxLength = this.maxLength ?? binaryData.byteLength; + this.maxLength = Math.max( + binaryData.byteLength, + this.maxLength ?? binaryData.byteLength + ); return super.writeBytes( stream, binaryData, From c4ed8fdde0a6fe2f106fbe35bac6df659a06efb3 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 17:47:32 -0400 Subject: [PATCH 21/22] feat(vr-refactors) Made sure that the _storeRaw acts as returnRaw for the purposes of reading a file. This accommodates the way data-tests checks against raw values. I agree that if a user wants to force a raw read, they should. Signed-off-by: Luis M. Santos --- src/ValueRepresentation.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ValueRepresentation.js b/src/ValueRepresentation.js index 0387891f..a15a472b 100644 --- a/src/ValueRepresentation.js +++ b/src/ValueRepresentation.js @@ -296,7 +296,7 @@ class ValueRepresentation { readPaddedAsciiString(stream, length) { if (!length) return ""; - if (stream.peekUint8(length - 1) !== this._padByte) { + if (stream.peekUint8(length - 1) !== this._padByte || this._storeRaw) { return stream.readAsciiString(length); } else { var val = stream.readAsciiString(length - 1); @@ -309,8 +309,9 @@ class ValueRepresentation { if (!length) return ""; const val = stream.readEncodedString(length); if ( - val.length && - val[val.length - 1] !== String.fromCharCode(this._padByte) + (val.length && + val[val.length - 1] !== String.fromCharCode(this._padByte)) || + this._storeRaw ) { return val; } else { From dbabdb5c1f559459282567a83e3015300dfc6b75 Mon Sep 17 00:00:00 2001 From: "Luis M. Santos" Date: Wed, 11 Mar 2026 17:48:20 -0400 Subject: [PATCH 22/22] feat(vr-tests) updated the tests in data-tests to reflect the correct raw value when accounting for oddness and pad byte value. Signed-off-by: Luis M. Santos --- test/data.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/data.test.js b/test/data.test.js index 3211f2cf..fb68472a 100644 --- a/test/data.test.js +++ b/test/data.test.js @@ -1457,11 +1457,11 @@ describe("test_un_vr", () => { 0x65, 0x5e, 0x4a, 0x61, 0x79, 0x5e, 0x41, 0x5e, 0x4a, 0x72, 0x2e, 0x20 ]).buffer, - ["Doe^John^A^Jr.^MD=Doe^Jay^A^Jr."], + ["Doe^John^A^Jr.^MD=Doe^Jay^A^Jr. "], [ { Alphabetic: "Doe^John^A^Jr.^MD", - Ideographic: "Doe^Jay^A^Jr." + Ideographic: "Doe^Jay^A^Jr. " } ] ], @@ -1527,7 +1527,7 @@ describe("test_un_vr", () => { 0x31, 0x2e, 0x32, 0x2e, 0x38, 0x34, 0x30, 0x2e, 0x31, 0x30, 0x30, 0x30, 0x38, 0x2e, 0x31, 0x2e, 0x32, 0x2e, 0x31 ]).buffer, - ["1.2.840.10008.1.2.1"], + ["1.2.840.10008.1.2.1\0"], ["1.2.840.10008.1.2.1"] ], [