search

samuel-lucas6 / Milva

A simple, cross-platform command line tool for hashing files and text.
https://samuellucas.com/Milva
GNU General Public License v3.0
10 stars 1 forks source link

Hashing Text #2

Closed BluePositive closed 2 years ago

BluePositive commented 2 years ago

Does it make sense to add the functionality to hash text?

I think it can be added fairly easily by adding a t/text bool flag (with false as the default) and passing it to the HashEachFile method and changing the GetHash method to accept a Stream instead of a FileStream and then in the HashEachFile method if the Text flag is true in the forEach loop we can convert the text to a Stream.

If you think it makes sense I can work on it.

samuel-lucas6 commented 2 years ago

I suppose the question is what's the use case for this? As far as I know, the hashing functionality on Windows only supports files, and it's possible to create a text file to hash text that way. There would need to be a separate option for text.

BluePositive commented 2 years ago

The use case for me is when I want to show someone how hashing works I usually find an online text to sha256 converter and show them that when typing "Hello" the hash is... and when I change the text to "hello" it changes the hash completely, it will help if I can just do it locally. That is just my (stupid) use case, but I think it could also be useful when you have a list of words and you want to see if 1 of these words matches some hash.

I didn't understand what you mean by

Windows only supports files,

all the hashing algorithms that you are using (in the GetHash method) are accepting a Stream and a string can easily be converted to a Stream. I actually played around with it a little and it was working for me on Windows.

About a separate option for text I think we don't need to add only 1 IsText flag and if we came up with a good name for the FilePaths property (better than FilePathsOrText 😏) we don't need to add more else if statements to the OnExecute method we just need to pass in the IsText flag to the HashEachFile method and have an if(isText) statement inside the HashEachFile method to convert the text to a Stream.

The most challenging part of this feature looks like changing the variables/methods names.

samuel-lucas6 commented 2 years ago

The use case for me is when I want to show someone how hashing works I usually find an online text to sha256 converter and show them that when typing "Hello" the hash is... and when I change the text to "hello" it changes the hash completely, it will help if I can just do it locally. That is just my (stupid) use case, but I think it could also be useful when you have a list of words and you want to see if 1 of these words matches some hash.

I know what you mean, but that could be done using a text file; it would just be more inconvenient. That second use case would probably require a custom program/script that reads each line in a text file.

I didn't understand what you mean by Windows only supports files

Sorry, that wasn't very clear. I'm referring to certutil and PowerShell. I made this tool to simplify such usage and to support newer algorithms. Ideally, the UX should be as simple as possible.

About a separate option for text I think we don't need to add only 1 IsText flag and if we came up with a good name for the FilePaths property (better than FilePathsOrText 😏) we don't need to add more else if statements to the OnExecute method we just need to pass in the IsText flag to the HashEachFile method and have an if(isText) statement inside the HashEachFile method to convert the text to a Stream.

But how will a file/directory name be distinguished from regular text? Or do you mean an option like --text?

BluePositive commented 2 years ago

About the benefit of this feature, I still think that it is a useful feature, and it won't make the UX for hashing files/folders more complex than it was before (because the UX won't change).

But how will a file/directory name be distinguished from regular text? Or do you mean an option like --text?

Sorry I was not clear. Obviously, we will need a --text flag, what I desired to say was that with the approach that I have in mind we won't need to add new else if statements to the OnExecute method to handle text. Maybe it is easier to understand when you look at the code I made a simple commit that implements the hash text feature with the approach that I have in mind. I didn't rename any variables/methods names so it should be easier to see the important changes.

Here is how it looks in action.

image

samuel-lucas6 commented 2 years ago

it won't make the UX for hashing files/folders more complex than it was before (because the UX won't change).

That's true; what I meant was that it would add another option.

Sorry I was not clear. Obviously, we will need a --text flag, what I desired to say was that with the approach that I have in mind we won't need to add new else if statements to the OnExecute method to handle text. Maybe it is easier to understand when you look at the code I made a simple commit that implements the hash text feature with the approach that I have in mind. I didn't rename any variables/methods names so it should be easier to see the important changes.

Yes, I see what you mean. The Stream idea is definitely nice, and thank you for the demo; I like your terminal. It's tempting. I think I'd implement it slightly differently, and I have a lot of things on my plate at the moment, but I suppose this wouldn't take long.

BluePositive commented 2 years ago

@samuel-lucas6 wow I didn't expect it to be done so quickly. I like to way you implement it nice job! Thanks a lot!

(Btw my terminal is tabby using PowerShell with posh-git)

samuel-lucas6 commented 2 years ago

Thank you and for the donation! I'll try and do a release soon. I'll look into that.