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

Closes #3096 Simple file system configuration change to sanitize uploaded filenames. #3180

Merged
merged 5 commits into from
Jun 12, 2024

Conversation

maine-inventor
Copy link
Contributor

@maine-inventor maine-inventor commented Feb 16, 2024

Description

Related issues

#3096

How to test

Add any node content that contains a media file field.
Select file upload and select a local image file with a whacky file name ( spaces, special characters)
[upload]
Observe the new file name all cleaned up. Example: spaces are now "-"s.
Also test the replacing of document files while preserving the filename (using the functionality from the Media Entity File Replace module).
Updated: Uploaded file character casing remains untouched.

Types of changes

Arizona Quickstart (install profile, custom modules, custom theme)

  • Patch release changes
    • Bug fix
    • Accessibility, performance, or security improvement
    • Critical institutional link or brand change
    • Adding experimental module
    • Update experimental module
  • Minor release changes
    • New feature
    • Breaking or visual change to existing behavior
    • Upgrade experimental module to stable
    • Enable existing module by default or database update
    • Non-critical brand change
    • New internal API or API improvement with backwards compatibility
    • Risky or disruptive cleanup to comply with coding standards
    • High-risk or disruptive change (requires upgrade path, risks regression, etc.)
  • Other or unknown
    • Other or unknown

Drupal core

  • Patch release changes
    • Security update
    • Patch level release (non-security bug-fix release)
    • Patch removal that's no longer necessary
  • Minor release changes
    • Major or minor level update
  • Other or unknown
    • Other or unknown

Drupal contrib projects

  • Patch release changes
    • Security update
    • Patch or minor level update
    • Add new module
    • Patch removal that's no longer necessary
  • Minor release changes
    • Major level update
  • Other or unknown
    • Other or unknown

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@maine-inventor maine-inventor linked an issue Feb 16, 2024 that may be closed by this pull request
@trackleft trackleft changed the title Simple file system configuration change to sanitize uploaded filenames. Closes ##3096 Simple file system configuration change to sanitize uploaded filenames. Feb 16, 2024
@trackleft trackleft changed the title Closes ##3096 Simple file system configuration change to sanitize uploaded filenames. Closes #3096 Simple file system configuration change to sanitize uploaded filenames. Feb 16, 2024
@bberndt-uaz
Copy link
Contributor

Do we want to enable all the "Sanitize filenames" settings? I'm wondering in particular about the "Convert to lowercase" setting.

@trackleft
Copy link
Member

trackleft commented Feb 16, 2024

Please commit the suggestion here: #3180 (comment)
image

Please convert this PR from a draft
image

To ready for review
image

@trackleft trackleft added enhancement New feature or request patch release Issues to be included in the next patch release and removed patch release Issues to be included in the next patch release labels Feb 16, 2024
@joeparsons
Copy link
Member

joeparsons commented Feb 21, 2024

Do we want to enable all the "Sanitize filenames" settings? I'm wondering in particular about the "Convert to lowercase" setting.

I do think it would be good to enable these settings by default on new site installs. We don't necessarily want to enable these settings on existing sites though.

@joeparsons
Copy link
Member

I was thinking we did this in Quickstart 1 but it looks like all we did was include the contrib Transliteration module but it wasn't installed by default. When that module was installed on Quickstart 1 its default settings basically match what is in this PR.

@maine-inventor maine-inventor marked this pull request as ready for review May 20, 2024 20:16
@maine-inventor maine-inventor requested a review from a team as a code owner May 20, 2024 20:16
Copy link
Contributor Author

@maine-inventor maine-inventor left a comment

Choose a reason for hiding this comment

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

Tested multiple filename patterns for upload(space, dash, underscore) , works as described

danahertzberg
danahertzberg previously approved these changes May 22, 2024
@danahertzberg
Copy link
Contributor

danahertzberg commented May 22, 2024

Check if this needs to be in the Quickstart folder. Test against existing site to ensure compatibility.

This is not a retroactive change. Will only work with new file uploads after this is merged.

Copy link
Member

@trackleft trackleft left a comment

Choose a reason for hiding this comment

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

Tested this, and it appears to apply the configuration update on sites that existed before this setting existed.

Site that I tested on began its life in drupal core 10.1.7
The setting edited in this PR was introduced in Drupal core 10.2.x

tadean
tadean previously approved these changes May 29, 2024
@trackleft trackleft added the needs-CWS-testing Needs manual pre/post release testing by Campus Web Services label May 29, 2024
@danahertzberg
Copy link
Contributor

danahertzberg commented May 29, 2024

Look into:

trackleft
trackleft previously approved these changes Jun 4, 2024
@maine-inventor maine-inventor dismissed stale reviews from trackleft, tadean, and danahertzberg via 8f1c6dd June 10, 2024 14:58
@maine-inventor
Copy link
Contributor Author

Updated the character casing check to not force to lower.

@trackleft trackleft merged commit 5b0c170 into main Jun 12, 2024
15 checks passed
@trackleft trackleft deleted the issue/3096 branch June 12, 2024 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.11.x only enhancement New feature or request needs-CWS-testing Needs manual pre/post release testing by Campus Web Services
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Configure filename sanitization
6 participants