-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
feat(load_config): enforce stricter extension checks #5591
base: master
Are you sure you want to change the base?
Conversation
How to testgit clone -b config https://github.com/hexojs/hexo.git
cd hexo
npm install
npm test |
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.
I still prefer to automatically choose config.yml
or config.json
without specifying it. We might wanna make findConfigPath
more strict though.
9c99da5
to
70f68d5
Compare
@SukkaW I've updated the code, and now the JSON suffix is also supported. |
}; | ||
|
||
async function findConfigPath(path: string): Promise<string> { | ||
const { dir, name } = parse(path); | ||
|
||
const files = await readdir(dir); | ||
const item = files.find(item => item.startsWith(name)); | ||
const item = files.find(item => item === name + '.json'); |
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.
This doesn't make sense. That should be using path.basename(item) === 'config'
.
What does it do?
I've noticed that Hexo supports searching for configuration files with different suffixes, such as
_config.txt
and_config.json
. However, this check is very loose, meaning that_config.backup
or_config.yml.bak
are also considered valid configuration files. This feature can cause confusion and even security issues for users. Typically, users use_config.yml
only, and in cases of custom usage, they can specifyhexo.config_path
. This suggests that there is little need for custom suffixes. Therefore, I recommend removing this feature in the pull request.Screenshots
Pull request tasks