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

Jump to bottom

Desktop: Fixes #10946: Stop crashing HTML/MD importer when content has link with very long name #10947

Closed
wants to merge 12 commits into from
Closed

Desktop: Fixes #10946: Stop crashing HTML/MD importer when content has link with very long name #10947

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b1db308
wip adding new checks to md import
pedr Aug 30, 2024
deceb77
changing checks in favour of handling error
pedr Aug 30, 2024
7d25c00
handle error on node fsDriver stat
pedr Aug 30, 2024
f82c6d3
reverting changes
pedr Aug 30, 2024
f7cf69f
removing error handling from stat
pedr Sep 2, 2024
0223773
adding a new way to check if it is to a localfile
pedr Sep 2, 2024
07e7ff7
simplifying importLocalFile and removing unused function
pedr Sep 2, 2024
fbc7aea
Merge branch 'dev' into skip-creating-resource-from-uris
laurent22 Sep 4, 2024
3a8f417
Merge branch 'dev' into skip-creating-resource-from-uris
laurent22 Sep 10, 2024
706bdc0
Merge branch 'dev' into skip-creating-resource-from-uris
personalizedrefrigerator Sep 12, 2024
bf7e03b
remove repeated logic
pedr Sep 17, 2024
f49b092
Merge remote-tracking branch 'refs/remotes/origin/skip-creating-resou…
pedr Sep 17, 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
1 change: 1 addition & 0 deletions packages/lib/fs-driver-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export default class FsDriverNode extends FsDriverBase {
};
} catch (error) {
if (error.code === 'ENOENT') return null;
if (error.code === 'ENAMETOOLONG') return null;
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this logic should be here. The way stats work is that it either returns the file info, or null if the file doesn't exist. But in this case it exists, except it cannot be processed. So whatever we need to solve it's definitely not here that the fix should be

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The PR name made this more confusing, my bad.

These links are not local resources. The way that importer works is by reading the HTML for links and trying to check if they exist locally or not. When it is a will be a very long link like, or a mailto: to a support address with a template included, for example, fsDriver stat will throw the ENAMETOOLONG.

There isn't any way to file exist because ENAMETOOLONG since it means it is a name longer than the system supports.

Copy link
Owner

Choose a reason for hiding this comment

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

The problem is that a link is being fed to this function, so that should not happen. The current behaviour is correct - it crashes when invalid data is passed to this function. Now the fix is to prevent that data to get there in the first place

throw error;
}
}
Expand Down
7 changes: 7 additions & 0 deletions packages/lib/fsDriver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,11 @@ describe('fsDriver', () => {
await shim.fsDriver().findUniqueFilename(join(supportDir, 'this-file-does-not-exist.txt'), [join(supportDir, 'some-other-file.txt')]),
).toBe(join(supportDir, 'this-file-does-not-exist.txt'));
});

it('should handle cases where filename is too long in stat', async () => {
const fsDriver = new FsDriverNode();
const filename = `${'12345678'.repeat(32)}9`;

expect(await fsDriver.stat(`./${filename}`)).toBe(null);
});
});
Loading