Skip to content

Sanitize filenames prior to DSpace publication - #1536

Closed
jazairi wants to merge 2 commits into
mainfrom
etd-703
Closed

Sanitize filenames prior to DSpace publication#1536
jazairi wants to merge 2 commits into
mainfrom
etd-703

Conversation

@jazairi

@jazairi jazairi commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Why these changes are being introduced:

An end user reported that they could not download
a file from DSpace. We learned that this was
due to a decomposed unicode character, and found
an additional 9 theses that were broken due to
filenames with zero-width whitespace and en-dash
characters.

We've fixed the 10 published theses in ETD, but
we need a way to proactively address this in the
future.

Relevant ticket(s):

How this addresses that need:

This adds to the SQS Message model a method that
sanitizes filenames by removing the characters
mentioned above, plus a few other potential
culprits that we've not yet seen. (See side
effects.)

It avoids the more aggressive approach of changing
filenames via a lifecycle hook. That option was
considered, but a lower-intervention method
seemed preferable.

Side effects of this change:

This makes a few assumptions about potential
problematic characters that we've not yet seen
in the wild (e.g., hidden directional characters
and dash variants other than en-dash). It felt
useful to be proactive about this.

Developer

Accessibility
  • ANDI or WAVE has been run in accordance to our guide.
  • This PR contains no changes to the view layer.
  • New issues flagged by ANDI or WAVE have been resolved.
  • New issues flagged by ANDI or WAVE have been ticketed (link in the Pull Request details above).
  • No new accessibility issues have been flagged.
New ENV
  • All new ENV is documented in README.
  • All new ENV has been added to Heroku Pipeline, Staging and Prod.
  • ENV has not changed.
Approval beyond code review
  • UXWS/stakeholder approval has been confirmed.
  • UXWS/stakeholder review will be completed retroactively.
  • UXWS/stakeholder review is not needed.
Additional context needed to review

I ran rubocop in a separate commit, because there were several unrelated changes. To review the actual changeset, it's probably easiest to look at the first commit.

Code Reviewer

Code
  • I have confirmed that the code works as intended.
  • Any CodeClimate issues have been fixed or confirmed as
    added technical debt.
Documentation
  • The commit message is clear and follows our guidelines
    (not just this pull request message).
  • The documentation has been updated or is unnecessary.
  • New dependencies are appropriate or there were no changes.
Testing
  • There are appropriate tests covering any new functionality.
  • No additional test coverage is required.

Why these changes are being introduced:

An end user reported that they could not download
a file from DSpace. We learned that this was
due to a decomposed unicode character, and found
an additional 9 theses that were broken due to
filenames with zero-width whitespace and en-dash
characters.

We've fixed the 10 published theses in ETD, but
we need a way to proactively address this in the
future.

Relevant ticket(s):

- [ETD-703](https://mitlibraries.atlassian.net/browse/ETD-703)
- [ETD-702](https://mitlibraries.atlassian.net/browse/ETD-702)

How this addresses that need:

This adds to the SQS Message model a method that
sanitizes filenames by removing the characters
mentioned above, plus a few other potential
culprits that we've not yet seen. (See side
effects.)

It avoids the more aggressive approach of changing
filenames via a lifecycle hook. That option was
considered, but a lower-intervention method
seemed preferable.

Side effects of this change:

This makes a few assumptions about potential
problematic characters that we've not yet seen
in the wild (e.g., hidden directional characters
and dash variants other than en-dash). It felt
useful to be proactive about this.
This is in a separate commit because it touches several lines of code unrelated to the previous changeset
@mitlib
mitlib temporarily deployed to thesis-submit-pr-1536 August 17, 2026 17:28 Inactive
@jazairi

jazairi commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Tagging for review, as I'm pretty sure the failing checks are due to the latest GitHub outage.

@JPrevost JPrevost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So the concern is solely the filename we are storing in PG and has no relation to the object stored in S3? i.e. we don't need to address this at the point of file upload from departments, it is safe to address it solely at the publication level?

I don't see us also updating the filename being sent to preservation storage. I assume they will want to know the published file name even if we changed it from the submitted filename?

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 98.29% (+0.004%) from 98.286% — etd-703 into main

@jazairi

jazairi commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

@JPrevost Yes, I didn't see the point in updating the filename in PG. (I could be persuaded otherwise.) I didn't update the filenames for preservation because Archivematica normalizes them on ingest, but it's probably a good idea to be consistent in what we send to each system.

@jazairi

jazairi commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR to evaluate the possibility of storing normalized filenames in PG.

@jazairi jazairi closed this Aug 20, 2026
@jazairi
jazairi deleted the etd-703 branch August 20, 2026 20:07
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.

4 participants