falahati / WindowsFirewallHelper

A class library to manage the Windows Firewall as well as adding your program to the Windows Firewall Exception list.
MIT License
276 stars 72 forks source link

Fork using Vanara libs supporting lots of .NET versions #41

Open dahall opened 4 years ago

dahall commented 4 years ago

Not really an issue, just an offer. I have forked the code and changed out the underlying COM classes for those in Vanara.PInvoke.FirewallApi. This lib supports .NET 2-4.x, .NET Core 2.0-3.1, and .NET Std 2.0. I then added those same supported options to the WindowsFirewallHelper assembly. If you're interested, I'm happy to help add in and support in your master.

falahati commented 4 years ago

Hey David; happy to see you here; been using your TaskScheduler package for ages now.

Thanks for the port. However, forgive me for not seeing how these changes actually solve any issue or bring something new to the table.

This library is on NetStandard 2 now and therefore is useable by NetCore and other libraries targeting NetStandard. .Net2 support is interesting, especially since this library should in theory support Windows XP, however, I was thinking of removing this feature and all links to the legacy API anyway. All this along with the fact that this change unnecessary adds two new dependencies to the project (Vanara.PInvoke.FirewallApi and Vanara.PInvoke.Shared) while the whole ComPinvoke code for this library is quite small and compact, makes me hesitant.

Did you try it on NetCore2? Since CustomMarshaller and therefore EnumeratorToEnumVariantMarshaler is missing in NetCore2 and I can see that Vanara.PInvoke.FirewallApi actually uses CustomMarshaller for enumeration.

Can Vanara helps with #32 somehow?

dahall commented 4 years ago

However, forgive me for not seeing how these changes actually solve any issue or bring something new to the table.

You're right, nothing much new here. In fact, I started the port mostly so that I wouldn't have to write a ton of unit tests for my FirewallApi COM extraction. I have got this working on earlier .NET Core versions. You can find the implementation of EnumeratorToEnumVariantMarshaler here which you could bring right into your code base. It does simplify all the collection COM classes. Feel free to just steal that part.

You may also want to look at some of my optimizations in your helper classes. There were quite a few cases where a lot of code could be wrapped into a single Linq call.

Can Vanara help with #32 is somehow?

Vanara does have multi-version support for the EventLog class in Vanara.Compatibility.EventLog which you could use similar to what I did for my TaskScheduler package.

I was thinking of removing this feature and all links to the legacy API anyway.

I tried doing this for TaskScheduler and had significant backlash last year. There were many still using both the legacy interface on XP and .NET 2.0.

SCLD-AFrey commented 4 years ago

Can you make this work on remote machines also? Or just local? I have searched and cannot figure out how to use on remote.

falahati commented 4 years ago

Allow me to keep this issue open so I can take another deeper look into the changes and the Vanara library. Especially the EventLog package.

Can you make this work on remote machines also? Or just local? I have searched and cannot figure out how to use on remote.

Please consider opening a new issue about this. As far as I know, managing remote machines' firewall rules is possible through WMI and this library was only a COM wrapper since now. Adding this feature requires a lot of abstraction and obviously new pieces of code to works well. I will keep the new issue open as a feature request.

SCLD-AFrey commented 4 years ago

Allow me to keep this issue open so I can take another deeper look into the changes and the Vanara library. Especially the EventLog package.

Can you make this work on remote machines also? Or just local? I have searched and cannot figure out how to use on remote.

Please consider opening a new issue about this. As far as I know, managing remote machines' firewall rules is possible through WMI and this library was only a COM wrapper since now. Adding this feature requires a lot of abstraction and obviously new pieces of code to works well. I will keep the new issue open as a feature request.

Moving to new issue