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.

Sign up for GitHub

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

fix: protect null frontmatter.tags in TaskFile.ts #3082

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix: protect null frontmatter.tags in TaskFile.ts #3082

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e270a37
Protect null tags on frontmatter
evolutioned Sep 14, 2024
3579598
Merge pull request #1 from evolutioned/evolutioned-null-tags-on-front…
evolutioned Sep 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/Scripting/TasksFile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ export class TasksFile {
const rawFrontmatter = cachedMetadata.frontmatter;
if (rawFrontmatter !== undefined) {
this._frontmatter = JSON.parse(JSON.stringify(rawFrontmatter));
this._frontmatter.tags = parseFrontMatterTags(rawFrontmatter) ?? [];
// protect against null tags (happens in my vault)
if (this._frontmatter) {
this._frontmatter.tags = parseFrontMatterTags(rawFrontmatter) ?? [];
}
Comment on lines +22 to +25
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the following lines:

obsidian-tasks/src/Scripting/TasksFile.ts

Lines 11 to 12 in e0bb9bd

// Always make TasksFile.frontmatter.tags exist and be empty, even if no frontmatter present:
private readonly _frontmatter = { tags: [] } as any;

line 21 of this code is wrong, as it always overwrites this._frontmatter, including if JSON.parse(JSON.stringify(rawFrontmatter)) returns a null or undefined value.

I suspect that this always-overwriting is the root cause of the issue you have found.

And you have worked around it by only updating this._frontmatter.tags if this._frontmatter is valid.

But if this._frontmatter is not valid, the TasksFile object is left in an invalid state.

As such, I don't think I can accept this PR as-is.

A fuller fix would not overwrite the this._frontmatter value if JSON.parse(JSON.stringify(rawFrontmatter)) returns invalid data.

/CC @ilandikov for comment too...

}

if (Object.keys(cachedMetadata).length !== 0) {
Expand Down