Skip to content

Validate domains before adding them to the database via /api/list#1761

Merged
DL6ER merged 5 commits into
development-v6from
tweak/validate_domains
Nov 20, 2023
Merged

Validate domains before adding them to the database via /api/list#1761
DL6ER merged 5 commits into
development-v6from
tweak/validate_domains

Conversation

@DL6ER

@DL6ER DL6ER commented Nov 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

What does this implement/fix?

See title. Fixes #1760

Current development-v6 will allow you to add any string as domain, this PR validates domains before adding them.

Screenshot from 2023-11-18 12-48-03

Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): N/A

By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

Checklist:

  • The code change is tested and works locally.
  • I based my code and PRs against the repositories developmental branch.
  • I signed off all commits. Pi-hole enforces the DCO for all contributions
  • I signed all my commits. Pi-hole requires signatures to verify authorship
  • I have read the above and my PR is ready for review.

@DL6ER DL6ER requested a review from a team November 18, 2023 11:52
@DL6ER DL6ER linked an issue Nov 18, 2023 that may be closed by this pull request
Domain validation allows invalid characters #1760
Closed
@DL6ER
Hostnames (non-FQDN) are valid domains in the context of allow/deny d…
0474240 
…omains, too

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER force-pushed the tweak/validate_domains branch from 602bc7e to 0474240 Compare November 18, 2023 20:08
@yubiuser yubiuser changed the title Validate domains before adding the to the database via /api/list Validate domains before adding them to the database via /api/list Nov 18, 2023
yubiuser
yubiuser reviewed Nov 18, 2023
View reviewed changes
Comment thread src/tools/gravity-parseList.c Outdated
@DL6ER @yubiuser
Update src/tools/gravity-parseList.c
d8a452f 
Co-authored-by: yubiuser <ckoenig@posteo.de>
Signed-off-by: DL6ER <DL6ER@users.noreply.github.com>
yubiuser
yubiuser reviewed Nov 19, 2023
View reviewed changes

@yubiuser yubiuser left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a hint that users need to enter punycode domain?

Screenshot at 2023-11-19 21-03-51

Or we do as in v5: convert domains to punycode ourselfs.

@DL6ER
Convert IDNs to punycode before validation
abbcbda 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER

DL6ER commented Nov 19, 2023

Copy link
Copy Markdown
Member Author

@yubiuser Added IDN conversion for domains being added - this makes me wonder if the web interface should show the UTF-8 form when the stored domain is punycode-encoded?...

Screenshot from 2023-11-19 22-35-44

(this is ä.com)

@yubiuser

yubiuser commented Nov 19, 2023

Copy link
Copy Markdown
Member

You mean like this (v5 screenshot)
Screenshot at 2023-11-19 22-41-17

@DL6ER
Add "unicode" field to API response for GET /api/domain
7f0a0a7 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER

DL6ER commented Nov 19, 2023

Copy link
Copy Markdown
Member Author

The latest commit added the necessary punycode -> Unicode back-conversion when GETting the domains. Achieving what is in your screenshot needs a subsequent web PR. However, the result can already now be seen in the network tab of your browser's dev tools:

Screenshot from 2023-11-19 22-47-22

@DL6ER DL6ER mentioned this pull request Nov 19, 2023
Merged
5 tasks
@DL6ER

DL6ER commented Nov 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

Corresponding web PR: pi-hole/web#2857

Screenshot from 2023-11-19 23-01-17

@DL6ER DL6ER mentioned this pull request Nov 20, 2023
Merged
5 tasks
@DL6ER DL6ER merged commit 1f320f8 into development-v6 Nov 20, 2023
@DL6ER DL6ER deleted the tweak/validate_domains branch November 20, 2023 18:43
@PromoFaux PromoFaux mentioned this pull request Feb 18, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yubiuser yubiuser yubiuser left review comments

@rdwebdesign rdwebdesign rdwebdesign approved these changes

Assignees

No one assigned

Labels

Pi-hole v6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Domain validation allows invalid characters

3 participants

@DL6ER @yubiuser @rdwebdesign