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

FEAT(client): Improve handling of IP masks in BanEditor #5406

Conversation

Krzmbrzl
Copy link
Member

@Krzmbrzl Krzmbrzl commented Jan 7, 2022

When using the BanEditor to manually create a new Ban entry, it was
really easy to get the "mask" property wrong, which had the effect of
apparently not performing any action at all, when clicking on the "Add"
button.

The underlying problem was most likely that the "mask" field defaults to
128, which is only valid for IPv6 addresses. When entering an IPv4
address, the mask would be considered invalid (through some internal
wizardry) and thus the Ban would not be added.
However, the user was not presented with any kind of error message that
would have hinted at the problem.

This commit tackles this issue by first of all validating that the
entered IP address is valid. If it is not, then the respective input
field will be colored red. If it is, the mask spin box's maximum value
will automatically be set to the correct maximum value for the used IP
address flavor (32 for IPv4 and 128 for IPv6).

That way the user can only ever press "Add", if they entered a valid IP
address alongside a valid mask. Thus, pressing "Add" should now always
add the respective ban.

Fixes #3986

Checks

When using the BanEditor to manually create a new Ban entry, it was
really easy to get the "mask" property wrong, which had the effect of
apparently not performing any action at all, when clicking on the "Add"
button.

The underlying problem was most likely that the "mask" field defaults to
128, which is only valid for IPv6 addresses. When entering an IPv4
address, the mask would be considered invalid (through some internal
wizardry) and thus the Ban would not be added.
However, the user was not presented with any kind of error message that
would have hinted at the problem.

This commit tackles this issue by first of all validating that the
entered IP address is valid. If it is not, then the respective input
field will be colored red. If it is, the mask spin box's maximum value
will automatically be set to the correct maximum value for the used IP
address flavor (32 for IPv4 and 128 for IPv6).

That way the user can only ever press "Add", if they entered a valid IP
address alongside a valid mask. Thus, pressing "Add" should now always
add the respective ban.

Fixes mumble-voip#3986
@Krzmbrzl Krzmbrzl added client feature-request This issue or PR deals with a new feature ui labels Jan 7, 2022
@Krzmbrzl Krzmbrzl mentioned this pull request Jan 7, 2022
@Krzmbrzl Krzmbrzl merged commit 53ddee7 into mumble-voip:master Jan 7, 2022
@Krzmbrzl Krzmbrzl deleted the feat-improve-mask-handling-in-ban-editor branch November 9, 2022 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client feature-request This issue or PR deals with a new feature ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to create ban
1 participant