NIAD-3252: External attachment filename containing equals sign throws…

… exception (#198) * add unit test for equals sign in file name when parsing ebxml * add fix to split to handle an equal sign in the filename. * Update CHANGELOG.md
NHSDigital · Dec 13, 2024 · 29a5841 · 29a5841
1 parent 8079d2b
commit 29a5841
Show file tree

Hide file tree

Showing 3 changed files with 160 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix issue where an error occurred if the attachment filenames contained and equal sign.
+
 ## [1.3.2] - 2024-09-09
 
 ### Fixed

diff --git a/mhs/common/mhs_common/messages/ebxml_envelope.py b/mhs/common/mhs_common/messages/ebxml_envelope.py
@@ -227,15 +227,21 @@ def parse_external_attachments(xml_tree: Element) -> Dict[str, str]:
                     variables = descriptionParams = re.findall("(?:\".*?\"|\S)+", description.strip())
 
                     filename = None
-                    description_variables = dict(pair.split("=") for pair in variables)
+
+                    description_variables = {}
+                    for pair in variables:
+                        if "=" in pair:
+                            key, value = pair.split("=", 1)
+                            description_variables[key] = value
+
                     if "Filename" in description_variables:
                         filename = description_variables["Filename"].replace('\\', '')
-                    
+
                     mid = mid_attribute.split(":")[1]
-                    external_attachment =  { 
+                    external_attachment = {
                         EXTERNAL_ATTACHMENT_DOCUMENT_ID :document_id_attribute, 
                         EXTERNAL_ATTACHMENT_MESSAGE_ID :mid_attribute.split(":")[1], 
-                        EXTERNAL_ATTACHMENT_DESCRIPTION: description, 
+                        EXTERNAL_ATTACHMENT_DESCRIPTION: description,
                         EXTERNAL_ATTACHMENT_TITLE: filename 
                     }
 

diff --git a/mhs/common/mhs_common/messages/tests/test_ebxml_envelope.py b/mhs/common/mhs_common/messages/tests/test_ebxml_envelope.py
@@ -25,6 +25,7 @@
     ebxml_envelope.RECEIVED_MESSAGE_ID: "F106022D-758B-49A9-A80A-8FF211C32A43"
 }
 
+
 class BaseTestEbxmlEnvelope(TestCase):
     current_dir = os.path.dirname(os.path.abspath(__file__))
     message_dir = Path(current_dir) / MESSAGE_DIR
@@ -44,3 +45,148 @@ def test_cant_find_optional_text_value_during_parsing(self):
         )
 
         self.assertEqual({}, values_dict)
+
+    def test_filename_contains_equals_sign(self):
+        expected_external_attachment = {
+            'document_id': '_735BB673-D9C0-4B85-951E-98DD045C4713',
+            'message_id': 'E54DEC57-6BA5-40AB-ACD0-1E383209C034',
+            'description': 'Filename="735BB673-D9C0-4B85-951E-98DD045C4713_adrian=marbles2.BMP" '
+                           'ContentType=application/octet-stream Compressed=Yes LargeAttachment=No '
+                           'OriginalBase64=No Length=3345444',
+            'title': '"735BB673-D9C0-4B85-951E-98DD045C4713_adrian=marbles2.BMP"'
+        }
+
+        description = ('Filename="735BB673-D9C0-4B85-951E-98DD045C4713_adrian=marbles2.BMP" '
+                       'ContentType=application/octet-stream Compressed=Yes LargeAttachment=No OriginalBase64=No '
+                       'Length=3345444')
+
+        xml_tree = self.generate_soap_envelope_from_description(description)
+
+        external_attachments = ebxml_envelope.EbxmlEnvelope.parse_external_attachments(xml_tree)['external_attachments']
+
+        self.assertEqual(external_attachments[0], expected_external_attachment)
+
+    def test_description_does_not_contain_filename(self):
+        expected_external_attachment = {
+            'document_id': '_735BB673-D9C0-4B85-951E-98DD045C4713',
+            'message_id': 'E54DEC57-6BA5-40AB-ACD0-1E383209C034',
+            'description': 'ContentType=application/octet-stream Compressed=Yes LargeAttachment=No '
+                           'OriginalBase64=No Length=3345444',
+            'title': None
+        }
+
+        description = (
+            'ContentType=application/octet-stream Compressed=Yes LargeAttachment=No OriginalBase64=No '
+            'Length=3345444'
+        )
+
+        xml_tree = self.generate_soap_envelope_from_description(description)
+        external_attachments = ebxml_envelope.EbxmlEnvelope.parse_external_attachments(xml_tree)['external_attachments']
+
+        self.assertEqual(external_attachments[0], expected_external_attachment)
+
+    def test_description_contains_filename_in_uppercase(self):
+        expected_external_attachment = {
+            'document_id': '_735BB673-D9C0-4B85-951E-98DD045C4713',
+            'message_id': 'E54DEC57-6BA5-40AB-ACD0-1E383209C034',
+            'description': 'FILENAME="735BB673-D9C0-4B85-951E-98DD045C4713_adrian=marbles2.BMP" '
+                           'ContentType=application/octet-stream Compressed=Yes LargeAttachment=No '
+                           'OriginalBase64=No Length=3345444',
+            'title': None
+        }
+
+        description = (
+            'FILENAME="735BB673-D9C0-4B85-951E-98DD045C4713_adrian=marbles2.BMP" '
+            'ContentType=application/octet-stream Compressed=Yes LargeAttachment=No OriginalBase64=No '
+            'Length=3345444'
+        )
+
+        xml_tree = self.generate_soap_envelope_from_description(description)
+
+        external_attachments = ebxml_envelope.EbxmlEnvelope.parse_external_attachments(xml_tree)['external_attachments']
+
+        self.assertEqual(external_attachments[0], expected_external_attachment)
+
+    def test_description_is_not_a_valid_gp2gp_message(self):
+        expected_external_attachment = {
+            'document_id': '_735BB673-D9C0-4B85-951E-98DD045C4713',
+            'message_id': 'E54DEC57-6BA5-40AB-ACD0-1E383209C034',
+            'description': 'This is not a valid GP2GP Message',
+            'title': None
+        }
+
+        description = 'This is not a valid GP2GP Message'
+
+        xml_tree = self.generate_soap_envelope_from_description(description)
+
+        external_attachments = ebxml_envelope.EbxmlEnvelope.parse_external_attachments(xml_tree)['external_attachments']
+
+        self.assertEqual(external_attachments[0], expected_external_attachment)
+
+    def test_description_contains_duplicated_filename_key(self):
+        expected_external_attachment = {
+            'document_id': '_735BB673-D9C0-4B85-951E-98DD045C4713',
+            'message_id': 'E54DEC57-6BA5-40AB-ACD0-1E383209C034',
+            'description': 'Filename="marbles.BMP" Filename="marbles2.BMP" '
+                           'ContentType=application/octet-stream Compressed=Yes LargeAttachment=No '
+                           'OriginalBase64=No Length=3345444',
+            'title': '"marbles2.BMP"'
+        }
+
+        description = (
+            'Filename="marbles.BMP" Filename="marbles2.BMP" ContentType=application/octet-stream '
+            'Compressed=Yes LargeAttachment=No OriginalBase64=No Length=3345444'
+        )
+
+        xml_tree = self.generate_soap_envelope_from_description(description)
+
+        external_attachments = ebxml_envelope.EbxmlEnvelope.parse_external_attachments(xml_tree)['external_attachments']
+
+        self.assertEqual(external_attachments[0], expected_external_attachment)
+
+    @staticmethod
+    def generate_soap_envelope_from_description(description):
+        message = '''
+            <soap:Envelope xmlns:eb="http://www.oasis-open.org/committees/ebxml-msg/schema/msg-header-2_0.xsd"
+                       xmlns:hl7ebxml="urn:hl7-org:transport/ebxml/DSTUv1.0"
+                       xmlns:soap="http://schemas.xmlsoap.org/soap/envelope/">
+            <soap:Header>
+            <eb:MessageHeader eb:version="2.0" soap:mustUnderstand="1">
+            <eb:From>
+              <eb:PartyId eb:type="urn:nhs:names:partyType:ocs+serviceInstance">C88046-807354</eb:PartyId>
+            </eb:From>
+            <eb:To>
+              <eb:PartyId eb:type="urn:nhs:names:partyType:ocs+serviceInstance">P83007-822482</eb:PartyId>
+            </eb:To>
+            <eb:CPAId>d434c95b662b941a59f9</eb:CPAId>
+            <eb:ConversationId>A0000131-351C-452F-BEDB-3ED4759A0800</eb:ConversationId>
+            <eb:Service>urn:nhs:names:services:gp2gp</eb:Service>
+            <eb:Action>RCMR_IN030000UK07</eb:Action>
+            <eb:MessageData>
+              <eb:MessageId>F01D21CB-31A3-49C6-B0FC-A64C858149AA</eb:MessageId>
+              <eb:Timestamp>2024-12-10T08:02:33.405Z</eb:Timestamp>
+              <eb:TimeToLive>2024-12-10T14:17:33.405Z</eb:TimeToLive>
+            </eb:MessageData>
+            <eb:DuplicateElimination />
+            </eb:MessageHeader>
+            <eb:AckRequested eb:version="2.0" soap:mustUnderstand="1"
+                 soap:actor="urn:oasis:names:tc:ebxml-msg:actor:nextMSH" eb:signed="false" />
+            </soap:Header>
+            <soap:Body>
+              <eb:Manifest eb:version="2.0" soap:mustUnderstand="1">
+                <eb:Reference xlink:href="cid:[email protected]/EMISWeb/GP2GP2.2A"
+                      xmlns:xlink="http://www.w3.org/1999/xlink">
+                  <eb:Description xml:lang="en">RCMR_IN030000UK07</eb:Description>
+                  <hl7ebxml:Payload style="HL7" encoding="XML" version="3.0" />
+                </eb:Reference>
+                <eb:Reference xlink:href="mid:E54DEC57-6BA5-40AB-ACD0-1E383209C034"
+                      eb:id="_735BB673-D9C0-4B85-951E-98DD045C4713"
+                      xmlns:xlink="http://www.w3.org/1999/xlink">
+                <eb:Description xml:lang="en">%s</eb:Description>
+                </eb:Reference>
+              </eb:Manifest>
+            </soap:Body>
+            </soap:Envelope>''' % description
+
+        xml_tree = ElementTree.fromstring(message)
+        return xml_tree