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.

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

ExtensionService does not validate the content of addins files #1415

Closed
CharliePoole opened this issue Jun 10, 2024 · 2 comments · Fixed by #1463
Closed

ExtensionService does not validate the content of addins files #1415

CharliePoole opened this issue Jun 10, 2024 · 2 comments · Fixed by #1463
Assignees
Labels
Milestone

Comments

@CharliePoole
Copy link
Collaborator

Currently, we assume that every entry in the file, once blank lines and comments are removed, is either a pattern or a valid path. Patterns are expanded to paths while we are searching for extensions but paths are used directly.

Error handling for patterns seems pretty good: if no file is found matching it we log that fact but otherwise ignore it. For paths without wild cards, we throw if the directory or file is not found.

The error message given when a path is not found can be cryptic. For example, a file containing a comment in C# format (leading double slash) gave the message "Directory '' not found." Figuring this out took an inordinate amount of time, since the message didn't clearly point to the addins file.

Proposed resolution:

  1. Check each non-pattern (entries with without *) for validity as a path. If an invallid entry is found, throw an exception immediately so that the extension may be removed or the file fixed.
  2. Optionally, check each pattern as well by replacing each * with a valid name and validate that as a path. In this case, we may want to simply issue a warning, as the error won't necessarily lead to an exception. If possible, we should try to discriminate between errors which will cause an exception and those which will not.

Comments please?

@CharliePoole CharliePoole self-assigned this Aug 14, 2024
@CharliePoole CharliePoole added this to the 3.18.2 milestone Aug 14, 2024
@CharliePoole
Copy link
Collaborator Author

It looks like this will require some serious refactoring first and I think that this area of the code could use some cleanup anyway. I'll start on that path and see where it takes me.

@CharliePoole
Copy link
Collaborator Author

This issue has been resolved in version 3.18.2

The release is available on:
GitHub.
NuGet packages are also available NuGet.org and
Chocolatey Packages may be found at Chocolatey.org

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant