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

Detect invalid characters in file/directory names #116

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

mcantelon
Copy link
Contributor

Added logic to validate structure to return validation errors if a file or directory's name contains characters incompatible with Archivematica.

Copy link

codecov bot commented Jan 22, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 59.97%. Comparing base (8d59bdc) to head (0dd3aa8).

Files with missing lines Patch % Lines
internal/activities/validate_structure.go 83.33% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
+ Coverage   59.77%   59.97%   +0.19%     
==========================================
  Files          39       39              
  Lines        2603     2616      +13     
==========================================
+ Hits         1556     1569      +13     
  Misses        920      920              
  Partials      127      127              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mcantelon mcantelon force-pushed the dev/check-name-chars branch from 935049c to a498615 Compare January 22, 2025 07:17
@mcantelon mcantelon marked this pull request as draft January 22, 2025 07:18
@mcantelon mcantelon force-pushed the dev/check-name-chars branch 4 times, most recently from d2a330f to 0b76296 Compare January 22, 2025 07:34
@mcantelon mcantelon marked this pull request as ready for review January 22, 2025 07:36
Copy link
Contributor

@jraddaoui jraddaoui left a comment

Choose a reason for hiding this comment

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

Thanks @mcantelon!

@@ -180,6 +196,13 @@ func TestValidateStructure(t *testing.T) {
Failures: []string{"More than one dossier in the content directory"},
},
},
{
name: "Returns a failure when a SIP a directory name with an invalid character",
Copy link
Contributor

Choose a reason for hiding this comment

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

directory/file

Comment on lines 181 to 186
validChars := "-_.()"

// Generate string containing valid characters.
validChars = appendRuneRange(validChars, 'a', 'z')
validChars = appendRuneRange(validChars, 'A', 'Z')
validChars = appendRuneRange(validChars, '0', '9')
Copy link
Contributor

Choose a reason for hiding this comment

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

This is nice, but I think it would be a lot clearer with a long string constant. It will also prevent from calculating it each time.

@mcantelon
Copy link
Contributor Author

Thanks @jraddaoui ! PR updated!

Copy link
Contributor

@jraddaoui jraddaoui left a comment

Choose a reason for hiding this comment

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

Thanks @mcantelon!

func validateName(name string) bool {
const validChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_.()"

// Make sure only valid characters exist in name.
Copy link
Contributor

Choose a reason for hiding this comment

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

This type of comments are better above the function name:

// validateName makes sure only valid characters exist in name.
func validateName(name string) bool {

Not a big deal in this case for a small private function, but that way it documents the function and you can read it when you use it.

Added logic to validate structure to return validation errors if a file
or directory's name contains characters incompatible with
Archivematica.
@mcantelon mcantelon force-pushed the dev/check-name-chars branch from 0dd3aa8 to d763a0f Compare January 28, 2025 18:51
@mcantelon mcantelon merged commit 067b43b into main Jan 28, 2025
7 checks passed
@mcantelon mcantelon deleted the dev/check-name-chars branch January 28, 2025 18:54
@mcantelon
Copy link
Contributor Author

Thanks @jraddaoui !

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