Skip to content

fix(value-representation): prevent truncation of DA, DT, and TM in denormalization #427

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/DicomMetaDictionary.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,11 +270,19 @@ class DicomMetaDictionary {

if (!vr.isBinary() && vr.maxLength) {
dataItem.Value = dataItem.Value.map(value => {
if (value.length > vr.maxLength) {
let maxLength = vr.maxLength;
if (
vr.allowRangeMatching() &&
vr.rangeMatchingMaxLength != null
) {
maxLength = vr.rangeMatchingMaxLength;
}

if (value.length > maxLength) {
log.warn(
`Truncating value ${value} of ${naturalName} because it is longer than ${vr.maxLength}`
`Truncating value ${value} of ${naturalName} because it is longer than ${maxLength}`
);
return value.slice(0, vr.maxLength);
return value.slice(0, maxLength);
} else {
return value;
}
Expand Down
65 changes: 10 additions & 55 deletions src/ValueRepresentation.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ let DicomMessage, Tag;

var binaryVRs = ["FL", "FD", "SL", "SS", "UL", "US", "AT"],
explicitVRs = ["OB", "OW", "OF", "SQ", "UC", "UR", "UT", "UN", "OD"],
singleVRs = ["SQ", "OF", "OW", "OB", "UN"];
singleVRs = ["SQ", "OF", "OW", "OB", "UN"],
rangeMatchingVRs = ["DA", "DT", "TM"];

class ValueRepresentation {
constructor(type) {
Expand All @@ -78,6 +79,7 @@ class ValueRepresentation {
this._allowMultiple =
!this._isBinary && singleVRs.indexOf(this.type) == -1;
this._isExplicit = explicitVRs.indexOf(this.type) != -1;
this._allowRangeMatching = rangeMatchingVRs.includes(this.type) != -1;
this._storeRaw = true;
}

Expand All @@ -101,6 +103,10 @@ class ValueRepresentation {
return this._isExplicit;
}

allowRangeMatching() {
return this._allowRangeMatching;
}

/**
* Flag that specifies whether to store the original unformatted value that is read from the dicom input buffer.
* The `_rawValue` is used for lossless round trip processing, which preserves data (whitespace, special chars) on write
Expand Down Expand Up @@ -694,28 +700,11 @@ class DateValue extends AsciiStringRepresentation {
constructor(value) {
super("DA", value);
this.maxLength = 8;
this.rangeMatchingMaxLength = 18;
this.padByte = PADDING_SPACE;
//this.fixed = true;
this.defaultValue = "";
}

writeBytes(stream, value, writeOptions = {}) {
// Check if this is a query with range matching (contains a hyphen)
const isRangeQuery = typeof value === "string" && value.includes("-");

// For query with range matching, temporarily adjust maxLength
const originalMaxLength = this.maxLength;
if (isRangeQuery) {
this.maxLength = 18;
}

const result = super.writeBytes(stream, value, writeOptions);

// Restore original maxLength
this.maxLength = originalMaxLength;

return result;
}
}

class NumericStringRepresentation extends AsciiStringRepresentation {
Expand Down Expand Up @@ -796,26 +785,9 @@ class DateTime extends AsciiStringRepresentation {
constructor() {
super("DT");
this.maxLength = 26;
this.rangeMatchingMaxLength = 54;
this.padByte = PADDING_SPACE;
}

writeBytes(stream, value, writeOptions = {}) {
// Check if this is a query with range matching (contains a hyphen)
const isRangeQuery = typeof value === "string" && value.includes("-");

// For range queries, temporarily adjust maxLength
const originalMaxLength = this.maxLength;
if (isRangeQuery) {
this.maxLength = 54;
}

const result = super.writeBytes(stream, value, writeOptions);

// Restore original maxLength
this.maxLength = originalMaxLength;

return result;
}
}

class FloatingPointSingle extends ValueRepresentation {
Expand Down Expand Up @@ -1249,6 +1221,7 @@ class TimeValue extends AsciiStringRepresentation {
constructor() {
super("TM");
this.maxLength = 16;
this.rangeMatchingMaxLength = 28;
this.padByte = PADDING_SPACE;
}

Expand All @@ -1259,24 +1232,6 @@ class TimeValue extends AsciiStringRepresentation {
applyFormatting(value) {
return rtrim(value);
}

writeBytes(stream, value, writeOptions = {}) {
// Check if this is a query with range matching (contains a hyphen)
const isRangeQuery = typeof value === "string" && value.includes("-");

// For range queries, temporarily adjust maxLength
const originalMaxLength = this.maxLength;
if (isRangeQuery) {
this.maxLength = 28;
}

const result = super.writeBytes(stream, value, writeOptions);

// Restore original maxLength
this.maxLength = originalMaxLength;

return result;
}
}

class UnlimitedCharacters extends EncodedStringRepresentation {
Expand Down
63 changes: 63 additions & 0 deletions test/data.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,69 @@ it("test_custom_dictionary", () => {
expect(Object.keys(dataset).length).toEqual(17);
});

it("test_code_string_vr_truncated", () => {
// Create a dataset with a CS value that exceeds the 16-character limit and gets truncated
const testDataset = {
Modality: "MAGNETICRESONANCE"
};

const denaturalizedDataset =
DicomMetaDictionary.denaturalizeDataset(testDataset);

expect(denaturalizedDataset["00080060"].vr).toEqual("CS");
expect(denaturalizedDataset["00080060"].Value[0]).toEqual(
"MAGNETICRESONANC"
);
});

it("test_date_time_vr_range_matching_not_truncated", () => {
// Create a dataset with DT (DateTime) value representation with range matching
const testDataset = {
// 2023-01-31 08:30 AM to 09:00 AM UTC
DateTime: "20230131083000.000+0000-20230131090000.000+0000"
};

const denaturalizedDataset =
DicomMetaDictionary.denaturalizeDataset(testDataset);

expect(denaturalizedDataset["0040A120"].vr).toEqual("DT");
expect(denaturalizedDataset["0040A120"].Value[0]).toEqual(
"20230131083000.000+0000-20230131090000.000+0000"
);
});

it("test_date_vr_range_matching_not_truncated", () => {
// Create a dataset with DA (Date) value representation with range matching
const testDataset = {
// January 1, 2023 to March 1, 2023
StudyDate: "20230101-20230301"
};

const denaturalizedDataset =
DicomMetaDictionary.denaturalizeDataset(testDataset);

expect(denaturalizedDataset["00080020"].vr).toEqual("DA");
expect(denaturalizedDataset["00080020"].Value[0]).toEqual(
"20230101-20230301"
);
});

it("test_time_range_matching_attribute_not_truncated", () => {
// Create a dataset with TM (Time) value representation with range matching
const testDataset = {
// 08:00 AM to 02:30 PM
StudyTime: "080000.000-143000.000"
};

const denaturalizedDataset =
DicomMetaDictionary.denaturalizeDataset(testDataset);

expect(denaturalizedDataset["00080030"].vr).toEqual("TM");
expect(denaturalizedDataset["00080030"].Value[0]).toEqual(
"080000.000-143000.000"
);
});

it("Reads DICOM with multiplicity", async () => {
const url =
"https://github.com/dcmjs-org/data/releases/download/multiplicity/multiplicity.dcm";
Expand Down