-
Notifications
You must be signed in to change notification settings - Fork 21
feat: add filename validation to newArticles command #289
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import { | ||
validateFilename, | ||
ERROR_FILENAME_EMPTY, | ||
ERROR_INVALID_CHARACTERS, | ||
ERROR_INVALID_START_END, | ||
} from "./filename-validator"; | ||
|
||
describe("validateFilename", () => { | ||
it.each([ | ||
"test", | ||
"test-article", | ||
"test_article", | ||
"test123", | ||
"記事テスト", | ||
"article.backup", | ||
"my-awesome-article", | ||
"test file", | ||
])("should accept valid filename '%s'", (name) => { | ||
const result = validateFilename(name); | ||
expect(result.isValid).toBe(true); | ||
expect(result.error).toBeUndefined(); | ||
}); | ||
|
||
it.each(["", " ", " ", "\t", "\n"])( | ||
"should reject empty or whitespace-only filename '%s'", | ||
(name) => { | ||
const result = validateFilename(name); | ||
expect(result.isValid).toBe(false); | ||
expect(result.error).toBe(ERROR_FILENAME_EMPTY); | ||
}, | ||
); | ||
|
||
it.each([ | ||
"test<file", | ||
"test>file", | ||
"test:file", | ||
'test"file', | ||
"test/file", | ||
"test\\file", | ||
"test|file", | ||
"test?file", | ||
"test*file", | ||
"test\x00file", | ||
])("should reject filename with invalid characters '%s'", (name) => { | ||
const result = validateFilename(name); | ||
expect(result.isValid).toBe(false); | ||
expect(result.error).toBe(ERROR_INVALID_CHARACTERS); | ||
}); | ||
|
||
it.each([".test", "test.", " test", "test ", "..test", "test.."])( | ||
"should reject filename starting or ending with dots or spaces '%s'", | ||
(name) => { | ||
const result = validateFilename(name); | ||
expect(result.isValid).toBe(false); | ||
expect(result.error).toBe(ERROR_INVALID_START_END); | ||
}, | ||
); | ||
}); |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. また、 export const validateFilename = (filename: string): string[] => {
const validators = [
// ...
];
return getValidationErrorMessages(filename, validators);
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// eslint-disable-next-line no-control-regex -- include control characters | ||
const INVALID_FILENAME_CHARS = /[<>:"/\\|?*\x00-\x1f]/; | ||
|
||
export const ERROR_FILENAME_EMPTY = "Filename is empty"; | ||
export const ERROR_INVALID_CHARACTERS = | ||
'Filename contains invalid characters: < > : " / \\ | ? * and control characters'; | ||
export const ERROR_INVALID_START_END = | ||
"Filename cannot start or end with a dot or space"; | ||
|
||
export interface FilenameValidationResult { | ||
isValid: boolean; | ||
error?: string; | ||
} | ||
|
||
export function validateFilename(filename: string): FilenameValidationResult { | ||
if (!filename || filename.trim().length === 0) { | ||
return { | ||
isValid: false, | ||
error: ERROR_FILENAME_EMPTY, | ||
}; | ||
} | ||
|
||
if (INVALID_FILENAME_CHARS.test(filename)) { | ||
return { | ||
isValid: false, | ||
error: ERROR_INVALID_CHARACTERS, | ||
}; | ||
} | ||
|
||
if (/^[. ]|[. ]$/.test(filename)) { | ||
return { | ||
isValid: false, | ||
error: ERROR_INVALID_START_END, | ||
}; | ||
} | ||
|
||
return { | ||
isValid: true, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Currently the command skips invalid filenames but always exits with success. Consider setting
process.exitCode = 1
when any validation fails so calling processes detect an overall failure.Copilot uses AI. Check for mistakes.