search

dokan-dev / dokan-dotnet

Dokan DotNet Wrapper
http://dokan-dev.github.io
MIT License
462 stars 116 forks source link

Fixed a crash in `ReadFileProxy` when debug logging is enabled #327

Closed lostmsu closed 1 year ago

lostmsu commented 1 year ago

The call ends up looking like this:

logger.Debug("\tContext\t{FileStream, ...}")

And the Debug interprets it as a format string, and the part of the string between curly braces gets incorrectly interpreted as a placeholder specification.

IMHO, a mature logging library should be used instead like Microsoft.Extensions.Logging

Or at the very least to prevent an issue like this there should be an overload without params object[] args which would have been selected instead, and which would take the string as the final message rather than format string.

lostmsu commented 1 year ago

Workaround until this is released is to not use any of the standard loggers or use DokanNet.Logging.Logger with debug action set to null.

Liryna commented 1 year ago

Thanks for the pull request @lostmsu ! That makes me think we should migrate all the logs in this file as you did 😢 since it could also happen for other calls. Do you mind doing the change in this pull request ? That would be very appreciated!

lostmsu commented 1 year ago

@Liryna I thought of that, and skimmed the file a bit, and the instances that you see in my PR are the only ones I noticed.

lostmsu commented 1 year ago

@Liryna skimmed again, found and fixed 1 other instance.

Liryna commented 1 year ago

Awesome @lostmsu ! Thank you very much for finding and providing the fix 🏆 Do you need me to make a release with the fix ?

lostmsu commented 1 year ago

No, I am using the workaround for now.

lostmsu commented 1 year ago

Lol I just hit this again in a new project and spent >30 mins on debugging again :-D

Perhaps a release would be nice.

The error in the debug output was "Throw: Input string was not in a correct format"