search

RamonGiovane / guiptables

A Graphic User Interface for Linux's Iptables Firewall. Made with Cockpit for CentOS
GNU Lesser General Public License v2.1
14 stars 3 forks source link

Fixed <link> to font-awesome css & Make chain filter list dynamically show all chain names in use #5

Closed CrazyKidJack closed 3 months ago

CrazyKidJack commented 3 months ago

This fixes Issue 4: Icons not showing up properly

I just changed the meta tag and it works again.

Additionally, I added a feature so that the chain filter list will show all defined chains by dynamically pulling the names from the rules into the list. image

Sorry normally I would have split these into separate PRs, but I accidentally forgot to create separate branches and figured these changes were both small enough that while logically separate, hopefully it wouldn't be too difficult to review them together

CrazyKidJack commented 3 months ago

FYI, I made a release on my fork of this repo with the mentioned changes. I'm running this on debian so my release has .deb packages... but all you'd have to do is convert to .rpm to install it

RamonGiovane commented 3 months ago

Hi! Thanks for contributing! I think this is a neat addition, although I experienced this error while running your branch on my Linux Mint:

image

On the main branch it doesn't crash. Those custom chains my have caused the bug: image

CrazyKidJack commented 3 months ago

@RamonGiovane Hey, thanks for responding. I'm not sure what you would like me to do though. Could you be more explicit? All you said was that you got an error (which I did not get myself). You didn't say what you found out about it, what you want to do / want me to do about it. Etc

  1. I understand that Cockpit is a RHEL project, so I'm not sure that errors that only show up on Debian-based systems are relevant. a. Do you experience Issue 4: Icons not showing up properly on RHEL-based systems? b. Do you experience the error you got on Mint when you run the code changes I submitted in the PR on RHEL-based systems?

  2. IF the error you mentioned is relevant (shows up on RHEL-based systems OR you are wanting to explicitly support non-standard installations on Debian-based systems) Are you wanting me to fix the error? Or are you working on fixing it yourself? Can you be more explicit about what you want from me?

  3. IF you want me to fix the error, I would be willing to do so, but I would need more information. First and foremost, I would need some kind of evidence that the changes I submitted in the PR are actually what caused the error. I'm fine with modifying my PR to fix the error so long as: a. the error is relevant to the purpose of the repo (see point 2) b. the error is caused by my code

  4. Assuming that the error is relevant to this repo AND caused by my code, I still don't think I have the relevant information to fix it. a. I don't have your iptables rules / configurations b. I don't know what is able to cause that error message or even what part of the code can generate the error. I could find these things, but if you could do some investigation into your error yourself and share that information with me, that would enable me (or you) to fix it faster c. I don't know how you installed it. For example, did you just install the .deb package on my repo directly onto your Mint system? Did you create your own package from the code in my repo and install that onto your Mint system? Did you build the PR from here into an .rpm package and then convert it to a .deb package and install on Mint? d. Did you try any debugging or investigation into the cause of the error? If so, what did you find? e. Maybe different browsers could behave differently since it's all client-side code? What browser are you using?

RamonGiovane commented 3 months ago

About your questions:

1 - This was a college project. In college we used to run CentOS for our experiments and stuff. Nowadays I pretty much don't use RHEL like systems anymore, but I guess Cockpit works well in any distro. a. I did not experienced the error on Mint, but I only tested placing the souce code on the Cockpit folder, not the binary release. b. I just download RHEL 9 to test it out.

2 - I'm sorry, I wasn't clear. I just made a comment to report something for us to investigate. In fact, I was investigating it myself and I just came back here now to write that it may not be related to your branch. After some browser refreshes the error was gone. Actually it seems to appear at will and I think I saw it on the main branch as well. I've been testing your modifications and some other stuff and I'm willing to merge it. Anyways, feel comfortable to contribute the way you want 😃

About support other distros, I think it's totally possible.

3 and 4 - Don't worry about it. I tried it myself and it's seems unrelated. As said in the topic above, I'm willing to merge it. I will just do a few more tests. I will let you know if I find any other weird behaiviour.

Thanks again. As I mentioned, this was a college project, we had sort of a tight schedule at the time, so it's far from perfect, but if you or anyone is interested on improve it, let's do it 👍

RamonGiovane commented 3 months ago

I found an error when deleting rules from chains with names that include the - character, such as: image

I fixed it. The - character was being used as separator in HTML components ids and classnames. Later on, these ids are splitted by the separator, so I made a commit changing this separator and improving a little bit the way this is done.

EDIT: I had to commit the fix to the main branch because I wasn't able to push to your branch. I'm not used to the Github colab system.

RamonGiovane commented 3 months ago

FYI: Testing in RHEL 9.3, I was able to reproduce the icons problem: image

Checking out to your branch it's working again: image

CrazyKidJack commented 3 months ago

Awesome! Glad you figured out the problem and that my changes worked for you after all :)