DNS Rewrites, duplicates items... maybe bug

andylau004 commented 6 months ago

Prerequisites

[X] I have checked the Wiki and Discussions and found no answer
[X] I have searched other issues and found no duplicates
[X] I want to report a bug and not ask a question or ask for help
[X] I have set up AdGuard Home correctly and configured clients to use it. (Use the Discussions for help with installing and configuring clients.)

Platform (OS and CPU architecture)

Linux, AMD64 (aka x86_64)

Installation

GitHub releases or script from README

Setup

On one machine

AdGuard Home version

master

Action

DNS rewrites

Allows to easily configure custom DNS response for a specific domain name.

in this web ,, background code

// handleRewriteAdd is the handler for the POST /control/rewrite/add HTTP API.
func (d *DNSFilter) handleRewriteAdd(w http.ResponseWriter, r *http.Request) {

Why not use map to remove duplicates dns rewrites items. or before append ,,check if exist...

why not

???

Expected result

before append ,,check if exist...

Actual result

so many duplicates item.

Additional information and/or screenshots

No response

andylau004 commented 6 months ago

Has anyone ever been encounter this problem ???

IonCimpoiesu commented 4 months ago

@andylau004 -- Here to report the same behavior when adding new DNS rewrites via the API.

immagine

Deleting one record will also delete all duplicates, still this is a bug triggered when adding new DNS rewrites; I actaully took a look at the dashboard since sending POST request to add the same record multiple times returned 200 Successful

IonCimpoiesu commented 4 months ago

@andylau004 -- Here to report the same behavior when adding new DNS rewrites via the API.

Deleting one record will also delete all duplicates, still this is a bug triggered when adding new DNS rewrites; I actaully took a look at the dashboard since sending POST request to add the same record multiple times returned 200 Successful

I was expecting a HTTP 500 Internal Error Status, but was surprised to receive multiple 200 Successful; either way, until an official fix is released, I advised anyone who stumbles into it to first make a DNS rewrites GET request, create an encoded BASE64 of {{domain}}:{{answer}} for each listing in the list, and perform a comparison against the payload of the POST request, following the same encoding process; then decide if they want to update the record or perform a sequence of DELETE/ADD operations.

tnyeanderson commented 4 months ago

perform a sequence of DELETE/ADD operations.

Unfortunately, if my DELETEs succeed but my ADDs fail, I could be stuck with NXDOMAIN.

I'd prefer to add my new rewrites, then delete the old ones. But I also want to clean up duplicates. Seems like this is not reliably possible with the current bugs. If a rewrite already had duplicates, but I still want it enabled when I'm done, then I can't do my DELETEs last, since it will remove all the duplicates at once and I'd have to re-add the rewrite after. Additionally, if all rewrites for that domain have the same answer, and I want to keep that answer but remove the duplicates, then I'm forced to have a brief "downtime" where AdGuardHome returns NXDOMAIN until the rewrite can be re-added.

In my ideal world, if I POST a new rewrite which didn't already exist, I'd get 201. If I try to POST a rewrite that already exists, the server would not create a duplicate entry, and would return 200 (the desired rewrite rule exists--goal achieved).

The DELETE (well, in this case, a POST with /delete in the url) should also only delete the first match, IMO. I can use /list to find out how many times I need to call the delete endpoint to achieve my goal. But obviously if the first issue is fixed, then this becomes much less important.

AdguardTeam / AdGuardHome