Repository: pydicom/pydicom

Issue:
pydicom produces invalid DICOM files if ds.EncapsulatedDocument contains byte array of odd length
**Bug Description**
When inserting a byte array of odd length into the ds.EncapsulatedDocument field, and saving this as a DICOM file, the DICOM file produced is not valid. This happens because the resulting file produced also have an odd number of bytes in the (0042,0011) OB Encapsulated Document DICOM tag which is not allowed according to the DICOM sepcification for Value Fields, http://dicom.nema.org/dicom/2013/output/chtml/part05/chapter_7.html

**Expected behavior**
Either pydicom could through and error specifying that the ds.EncapsulatedDocument field should contain an array of even length, or it could fix the problem by add and extra zero byte to the end of the ds.EncapsulatedDocument byte array when the length is odd.

**Steps To Reproduce**
I have written the following pdf2dcm.py command line utility to mimic the behaviour of pdf2dcm in the dcmtk suite:

```python
# inspired by: https://github.com/rohithkumar31/pdf2dicom

import argparse
import pydicom

EncapsulatedPDFStorage = '1.2.840.10008.5.1.4.1.1.104.1'


def generate_dicom_from_pdf(input_file, output_file, zero_pad=True):
    file_meta = pydicom.dataset.Dataset()

    # FileMetaInformationGroupLength only gets rewritten when saved if present
    file_meta.FileMetaInformationGroupLength = 206

    file_meta.MediaStorageSOPClassUID = EncapsulatedPDFStorage

    file_meta.MediaStorageSOPInstanceUID = pydicom.uid.generate_uid(pydicom.uid.PYDICOM_ROOT_UID)

    # from: https://pydicom.github.io/pydicom/dev/reference/uid.html
    file_meta.TransferSyntaxUID = pydicom.uid.ExplicitVRLittleEndian

    pydicom.dataset.validate_file_meta(file_meta, enforce_standard=True)

    # see: http://dicom.nema.org/dicom/2013/output/chtml/part10/chapter_7.html
    preamble = b"\0" * 128

    ds = pydicom.dataset.FileDataset(output_file, {}, file_meta=file_meta, preamble=preamble)
    # ds.fix_meta_info()

    ds.is_little_endian = True
    ds.is_implicit_VR = False

    ds.SpecificCharacterSet = 'ISO_IR 100'

    import datetime
    dt = datetime.datetime.now()
    ds.InstanceCreationDate = dt.strftime('%Y%m%d')
    ds.InstanceCreationTime = dt.strftime('%H%M%S')  # ('%H%M%S.%f')

    ds.SOPClassUID = EncapsulatedPDFStorage
    ds.SOPInstanceUID = file_meta.MediaStorageSOPInstanceUID
    ds.StudyDate = None
    ds.AcquisitionDateTime = None
    ds.StudyTime = None
    ds.ContentTime = None
    ds.ContentDate = None
    ds.AccessionNumber = None
    ds.Modality = 'DOC'  # document
    ds.ConversionType = 'WSD'  # workstation
    ds.Manufacturer = None
    ds.ReferringPhysicianName = None
    ds.PatientName = None
    ds.PatientID = None
    ds.PatientBirthDate = None
    ds.PatientSex = None
    ds.StudyInstanceUID = pydicom.uid.generate_uid()
    ds.SeriesInstanceUID = pydicom.uid.generate_uid()
    ds.StudyID = None
    ds.SeriesNumber = 1
    ds.InstanceNumber = 1
    ds.BurnedInAnnotation = 'YES'
    ds.ConceptNameCodeSequence = None
    # ConceptNameCodeSequence also sets: ds.SequenceDelimitationItem
    ds.DocumentTitle = None

    with open(input_file, 'rb') as f:
        pdf_file_as_bytes = f.read()

    # DICOM Value Fields must according to the
    # specification be an even number of bytes, see:
    # http://dicom.nema.org/dicom/2013/output/chtml/part05/chapter_7.html
    if zero_pad and len(pdf_file_as_bytes) % 2 != 0:
        pdf_file_as_bytes += b"\0"

    ds.EncapsulatedDocument = pdf_file_as_bytes
    ds.MIMETypeOfEncapsulatedDocument = 'application/pdf'

    ds.save_as(output_file)


if __name__ == '__main__':
    parser = argparse.ArgumentParser()
    parser.add_argument('--disable-zero-padding', action='store_false')
    parser.add_argument('-i', '--input-file')
    parser.add_argument('-o', '--output-file')
    args = parser.parse_args()

    generate_dicom_from_pdf(args.input_file, args.output_file, args.disable_zero_padding)
```

To reproduce the problem the --disable-zero-padding parameter can be added, and a pdf file of odd number of bytes in length can be used as input to the program, this will then product an output DICOM file containing an odd number of bytes in the (0042,0011) OB Encapsulated Document DICOM tag, which can be checked using the dciodvfy validation tool from the dicom3tools package:

```bash
wget http://dicom.nema.org/medical/dicom/current/output/pdf/part05.pdf
ls -l part05.pdf # should be odd number of bytes, currently 4676213 for this file
python pdf2dcm.py --disable-zero-padding -i part05.pdf -o part05.dcm

(0x0042,0x0011) OB Encapsulated Document  - Error - Bad Value Length - not a multiple of 2 - VL is 0x475a75 should be 0x475a76
Error - Dicom dataset read failed
```

**Environment**

```bash
$ python -m pydicom.env_info

module       | version
------       | -------
platform     | Linux-5.13.0-7614-generic-x86_64-with-glibc2.31
Python       | 3.9.5 (default, Jun  4 2021, 12:28:51)  [GCC 7.5.0]
pydicom      | 2.2.0
gdcm         | _module not found_
jpeg_ls      | _module not found_
numpy        | 1.21.2
PIL          | _module not found_
pylibjpeg    | _module not found_
openjpeg     | _module not found_
libjpeg      | _module not found_
```


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1539?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) Report
> Merging [#1539](https://codecov.io/gh/pydicom/pydicom/pull/1539?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (dd16dc7) into [master](https://codecov.io/gh/pydicom/pydicom/commit/a125a02132c2db5ff5cad445e4722802dd5a8d55?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (a125a02) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pydicom/pydicom/pull/1539/graphs/tree.svg?width=650&height=150&src=pr&token=YciLV15Lwr&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)](https://codecov.io/gh/pydicom/pydicom/pull/1539?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)

```diff
@@           Coverage Diff           @@
##           master    #1539   +/-   ##
=======================================
  Coverage   97.42%   97.42%           
=======================================
  Files          66       66           
  Lines       10182    10185    +3     
=======================================
+ Hits         9920     9923    +3     
  Misses        262      262           
```


| [Impacted Files](https://codecov.io/gh/pydicom/pydicom/pull/1539?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) | Coverage Δ | |
|---|---|---|
| [pydicom/filewriter.py](https://codecov.io/gh/pydicom/pydicom/pull/1539/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9maWxld3JpdGVyLnB5) | `97.79% <100.00%> (+0.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1539?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1539?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom). Last update [a125a02...dd16dc7](https://codecov.io/gh/pydicom/pydicom/pull/1539?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).

mrbean-bremen: Ah, and can you please an entry in the release notes?


Patch:
diff --git a/doc/release_notes/index.rst b/doc/release_notes/index.rst
index 85f5be99c..4d47c7285 100644
--- a/doc/release_notes/index.rst
+++ b/doc/release_notes/index.rst
@@ -2,6 +2,7 @@
 Release notes
 =============
 
+.. include:: v2.3.0.rst
 .. include:: v2.2.0.rst
 .. include:: v2.1.1.rst
 .. include:: v2.1.0.rst
diff --git a/doc/release_notes/v2.3.0.rst b/doc/release_notes/v2.3.0.rst
new file mode 100644
index 000000000..743f24e81
--- /dev/null
+++ b/doc/release_notes/v2.3.0.rst
@@ -0,0 +1,13 @@
+Version 2.3.0
+=================================
+
+Changes
+-------
+
+Enhancements
+------------
+
+Fixes
+-----
+
+* Fixed odd-length **OB** values not being padded during write (:issue:`1511`)
diff --git a/pydicom/filewriter.py b/pydicom/filewriter.py
index 6f97b0e75..ab164a61b 100644
--- a/pydicom/filewriter.py
+++ b/pydicom/filewriter.py
@@ -272,7 +272,12 @@ def write_numbers(fp: DicomIO, elem: DataElement, struct_format: str) -> None:
 
 def write_OBvalue(fp: DicomIO, elem: DataElement) -> None:
     """Write a data_element with VR of 'other byte' (OB)."""
-    fp.write(cast(bytes, elem.value))
+    if len(elem.value) % 2:
+        # Pad odd length values
+        fp.write(cast(bytes, elem.value))
+        fp.write(b'\x00')
+    else:
+        fp.write(cast(bytes, elem.value))
 
 
 def write_OWvalue(fp: DicomIO, elem: DataElement) -> None:


Test patch:
diff --git a/pydicom/tests/test_filewriter.py b/pydicom/tests/test_filewriter.py
index 099d56307..dea21a238 100644
--- a/pydicom/tests/test_filewriter.py
+++ b/pydicom/tests/test_filewriter.py
@@ -474,6 +474,20 @@ class TestWriteDataElement:
         data_elem = DataElement(0x00080060, 'CS', b'REG')
         self.check_data_element(data_elem, expected)
 
+    def test_write_OB_odd(self):
+        """Test an odd-length OB element is padded during write"""
+        value = b'\x00\x01\x02'
+        elem = DataElement(0x7FE00010, 'OB', value)
+        encoded_elem = self.encode_element(elem)
+        ref_bytes = b'\xe0\x7f\x10\x00\x04\x00\x00\x00' + value + b"\x00"
+        assert ref_bytes == encoded_elem
+
+        # Empty data
+        elem.value = b''
+        encoded_elem = self.encode_element(elem)
+        ref_bytes = b'\xe0\x7f\x10\x00\x00\x00\x00\x00'
+        assert ref_bytes == encoded_elem
+
     def test_write_OD_implicit_little(self):
         """Test writing elements with VR of OD works correctly."""
         # VolumetricCurvePoints