-
Notifications
You must be signed in to change notification settings - Fork 153
A file is local if it exists in the root folder or a subfolder #72
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
Conversation
…with normalized paths. closes #71
if (typeof ref !== "string" || /^(\w+:)\/\//.test(ref)) return false; // absolute url | ||
ref = normalize(ref); | ||
return !ref.startsWith("../") && canReadSync(join(root, ref)) ? ref : false; | ||
} |
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.
thoughts on splitting this function into 2 - one for checking isLocalFile
and the other for returning the pathFromRoot
string? I was thinking
// A file is local if it exists in the root folder or a subfolder.
function isLocalFile(ref: string | null, root: string): boolean {
if (typeof ref !== "string || !canReadSync(join(root, ref))) return false;
return true;
}
function pathFromRoot(ref: string | null, root: string): string {
if (isLocalFile(ref, root))) return join(root, ref);
throw Error(`${ref} does not exist within the ${root} root directory`)
}
it's a little verbose but we can just use isLocalFile
in src/javascript.ts
join should already normalize the paths (according to the docs)
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.
instead of throwing an error we can just return null
for pathFromRoot
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.
thanks. I've applied your suggestions, still making sure that the normalized ref itself does not start with ../
, otherwise it would allow to reach ../public/database.js for example.
closes #71