Skip to content
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

NIAD-3252: External attachment filename containing equals sign throws exception #198

Merged

Conversation

MartinWheelerMT
Copy link
Contributor

@MartinWheelerMT MartinWheelerMT commented Dec 11, 2024

What

Updated the code using the split command in ebxml_envelope.py so that it only splits on the first occurence of = so that the full filename used and not attempted to be split further if it contains an =. This was also updated to not attempt to a process a provided variable if it is not a valid key value pair

Have also added tests to handle the following cases for the description:

  • Invalid Description: <eb:Description>This isn't a GP2GP message</eb:Description>
  • Duplicated key: <eb:Description>Filename=Value Filename=Value2</eb:Description>
  • Case-sensitivity of filename: <eb:Description>FILENAME=Value Filename=Value2</eb:Description>
  • Filename key-value pair is not provided

Why

If an attachment filename contains an equals sign (such as in the example below) then a error is currently thrown. This should be handled that a filename with an equals sign does not cause an issue.

<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">Filename="735BB673-D9C0-4B85-951E-98DD045C4713_adrian=marbles2.BMP" ContentType=application/octet-stream Compressed=Yes LargeAttachment=No OriginalBase64=No Length=3345444</eb:Description>
</eb:Reference>

Also, from discussion, the following checks were required to make sure the adaptor throw errors in the following cases:

  • Invalid Description: <eb:Description>This isn't a GP2GP message</eb:Description>
  • Duplicated key: <eb:Description>Filename=Value Filename=Value2</eb:Description>
  • Case-sensitivity of filename: <eb:Description>FILENAME=Value Filename=Value2</eb:Description>
  • Filename key-value pair is not provided

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the Changelog with details of my change in the UNRELEASED section if this change will affect end users

@MartinWheelerMT MartinWheelerMT force-pushed the niad-3252-external-attachment-equals-in-filename branch 3 times, most recently from 6939b5d to d120571 Compare December 11, 2024 16:04
@MartinWheelerMT MartinWheelerMT force-pushed the niad-3252-external-attachment-equals-in-filename branch 3 times, most recently from 70512f1 to 2e36a73 Compare December 11, 2024 16:36
@MartinWheelerMT MartinWheelerMT force-pushed the niad-3252-external-attachment-equals-in-filename branch 10 times, most recently from 41aeef0 to e0760f6 Compare December 11, 2024 18:36
@MartinWheelerMT MartinWheelerMT force-pushed the niad-3252-external-attachment-equals-in-filename branch from e0760f6 to e1e5e37 Compare December 11, 2024 18:40
@MartinWheelerMT MartinWheelerMT changed the title NIAD-3252: Manifest external attachment filename containing equals sign throws exception NIAD-3252: External attachment filename containing equals sign throws exception Dec 12, 2024
@MartinWheelerMT MartinWheelerMT marked this pull request as ready for review December 12, 2024 10:19
@MartinWheelerMT MartinWheelerMT enabled auto-merge (squash) December 12, 2024 10:20
CHANGELOG.md Outdated Show resolved Hide resolved
@MartinWheelerMT MartinWheelerMT force-pushed the niad-3252-external-attachment-equals-in-filename branch from 36395a9 to cdc91a8 Compare December 12, 2024 14:26
@MartinWheelerMT MartinWheelerMT force-pushed the niad-3252-external-attachment-equals-in-filename branch from 181a2c5 to 6a82af3 Compare December 12, 2024 14:57
…educe duplication.

Inline xml into the above method, and remove previously added xml file.
Add test to ensure that `title` is none when the description does not contain a filename.
Add test to ensure that title is set to none if case-sensitive 'Filename' is not provided as a key.
Add test to ensure that title is not set, but attachment is still handled when a description does not contain key value pairs.
Add test to ensure that the adaptor hands duplicated `Filename` keys by using the value from the last key.

Add functionality to ebxml_envelope so that a check is completed on each description variable to ensure that it is a valid key value pair (`key=value`).
Copy link
Collaborator

@adrianclay adrianclay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor feedback is that grouping the tests related to filenames under a common class might make it a bit more readable. May be easier to chat through my thinking though.

@MartinWheelerMT MartinWheelerMT merged commit 29a5841 into main Dec 13, 2024
1 check passed
@MartinWheelerMT MartinWheelerMT deleted the niad-3252-external-attachment-equals-in-filename branch December 13, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants