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

Upgrade the project to .net 8.0 and fix high security vulnerabilities (one failing test remaining) #548

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

DamirTomic
Copy link

Hi, I wanted to fix a dependency that is rated a high-security vulnerability, but in order to do that I had to make significant upgrades to the project, such as upgrading it to .net 8.0. I hope you're ok with that, if not, we can discuss.

There is 1 test that's failing but I think the test is incorrectly written. I can fix it we agree on a solution
a) test shouldn't throw an exception if you pass a valid string but the file doesn't exist
b) test should throw a filenotfound exception or something like that if the string exists but the file path doesn't
c) other ?

@toddams
Copy link
Owner

toddams commented Dec 23, 2024

Hey Damir

Thans for your PR, it's very valuable missing piece we had. However, because you replaced tabs with spaces on certain files (but not all of them, though) - it completely messed up the diff, and it's hard to tell what was changed without going line by line.

Also, dropping netstandard is going to make library unusable for some people, which I'd like to avoid. Are you sure there is no way to have updated packages without vulnerabilities without dropping it?

I also noticed, that #if directives for runtime-specific areas were not changed
image

@DamirTomic
Copy link
Author

Ok, let me see if I can make this work with .net standard, and I'll fix the other things as well.

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

Successfully merging this pull request may close these issues.

2 participants