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

Improve the DHCP data validation on the UI #2842

Closed
ainar-g opened this issue Mar 18, 2021 · 2 comments
Closed

Improve the DHCP data validation on the UI #2842

ainar-g opened this issue Mar 18, 2021 · 2 comments

Comments

@ainar-g
Copy link
Contributor

ainar-g commented Mar 18, 2021

Context: #2541, #2838.

Currently, there are at least three issues:

  1. The frontend seems to only validate the last part of the range_start and range_end IPv4 addresses. It should instead calculate the difference properly and allow settings, for example, range_start to 1.2.3.4 and range_end to 1.2.4.1.

  2. The frontend should validate that the new static lease's IP address is within the subnet defined by the netmask.

  3. The frontend seems to reject MAC addresses without colons. This seems overly strict, as 112233AABBCC and 11:22:33:AA:BB:CC are technically the same. In fact, some hardware manufacturers use the first form instead of the second one.

    Note, that the frontend still should send the form with the colons to the backend.

@mc9662
Copy link

mc9662 commented Apr 6, 2021

Hi, I'm not 100% sure this is data validation related, but it may be part of that. I'm running Version: v0.106.0-b.1 on an RPI3

If I stop the service via sudo ./AdGuardHome -s stop

and manually set DHCPv4 Options in AdGuardHome.yaml to have a secondary DNS (I'm running 2 instances of AdGuardHome for failover) e.g.

DHCPv4
#
options:
- 6 ips 192.168.0.10,192.168.0.11

save the yaml, restart the service and make a change in the GUI e.g. change the DHCP lease time to any other value and click "save config" the settings I manually added to the yaml revert to

DHCPv4
#
options: []

The GUI seems to reset the yaml and discard any manual changes. It would be better if the GUI only changes the yaml sections that are editable within the GUI.

Thanks

Matt

@ainar-g
Copy link
Contributor Author

ainar-g commented Apr 6, 2021

@mc9662, thanks for reporting. I've filed a new issue about it. See #2927.

heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Closes AdguardTeam#2842.

Squashed commit of the following:

commit 8580db9
Merge: a5d7187 ab85ad5
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Apr 13 15:29:06 2021 +0300

    Merge branch 'master' into 2842-dhcp-validation-ui

commit a5d7187
Author: Ildar Kamalov <ik@adguard.com>
Date:   Tue Apr 13 15:08:28 2021 +0300

    fix: revert deleted translation

commit 5016926
Merge: 46adf2c 48d702f
Author: Ildar Kamalov <ik@adguard.com>
Date:   Tue Apr 13 14:39:40 2021 +0300

    Merge branch 'master' into 2842-dhcp-validation-ui

commit 46adf2c
Author: Ildar Kamalov <ik@adguard.com>
Date:   Tue Apr 13 14:13:12 2021 +0300

    fix: no-bitwise

commit 1afc403
Author: Ildar Kamalov <ik@adguard.com>
Date:   Tue Apr 13 13:57:43 2021 +0300

    fix: IPv4 in CIDR validation

commit 2035a3f
Author: Ildar Kamalov <ik@adguard.com>
Date:   Tue Apr 13 11:58:03 2021 +0300

    fix: translations

commit 6dd455f
Author: Ildar Kamalov <ik@adguard.com>
Date:   Tue Apr 13 11:57:27 2021 +0300

    fix: MAC validation

commit 281e49a
Merge: 48b50ce 65553a2
Author: Ildar Kamalov <ik@adguard.com>
Date:   Tue Apr 6 18:12:06 2021 +0300

    Merge branch 'master' into 2842-dhcp-validation-ui

commit 48b50ce
Author: Artem Baskal <a.baskal@adguard.com>
Date:   Mon Apr 5 19:04:35 2021 +0300

    Add leases ip validation

commit 8630f3b
Author: Artem Baskal <a.baskal@adguard.com>
Date:   Mon Apr 5 13:59:16 2021 +0300

    Add helper for subnet to bitmap mask conversion, write test

commit 80dc7a8
Author: Artem Baskal <a.baskal@adguard.com>
Date:   Fri Apr 2 17:46:27 2021 +0300

    2842 Update DHCP range validation in UI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants