Pull request: 1577: rewrite edit http api

Merge in DNS/adguard-home from 1577-rewrite-edit to master

Squashed commit of the following:

commit d03bee2a14337d169eea950b3df18a447c02b422
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri May 12 12:54:15 2023 +0300

    filtering: imp tests

commit bd68320df6dc057d922d91551cd00c74ebfaad6c
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri May 12 10:38:12 2023 +0300

    filtering: rewrite http tests

commit 0d8bbcd0194c0db89a6d4b45927669423c9bbb59
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu May 11 12:06:30 2023 +0300

    filtering: rewrite http tests

commit 29080384dd8fa80d5286d2fac1a4429d712bbafa
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 10 17:24:00 2023 +0300

    filtering: imp code

commit 96c6b1c98debfae565c5e6254746959a4307744e
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 10 16:56:32 2023 +0300

    filtering: imp code

commit b5d0c50ea11f9d829ba9d2b188fcc471a965e012
Merge: 5fa9e1c37 c77b2a0ce
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 10 16:37:19 2023 +0300

    Merge remote-tracking branch 'origin/master' into 1577-rewrite-edit

commit 5fa9e1c3714e107f893c03efa72227f3ed88691c
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 10 16:35:19 2023 +0300

    filtering: imp code

commit dd9dce8fbf0ce4bd200f2fc2fbf580e025920cd5
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 10 13:06:43 2023 +0300

    docs: rewrite http update

commit 0c67b040e80787b084c4669bb20db8d6d145fc1b
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 10 13:04:31 2023 +0300

    filtering: rewrite http update

CHANGELOG.md

@@ -23,6 +23,11 @@ See also the [v0.107.30 GitHub milestone][ms-v0.107.30].
 NOTE: Add new changes BELOW THIS COMMENT.
 -->
 
+### Added
+
+- The ability to edit rewrite rules via `PUT /control/rewrite/update` HTTP API
+  ([#1577]).
+
 ### Fixed
 
 - Unquoted IPv6 bind hosts with trailing colons erroneously considered
@@ -35,6 +40,7 @@ NOTE: Add new changes BELOW THIS COMMENT.
 - Provided bootstrap servers are now used to resolve the hostnames of plain
   UDP/TCP upstream servers.
 
+[#1577]: https://github.com/AdguardTeam/AdGuardHome/issues/1577
 [#5716]: https://github.com/AdguardTeam/AdGuardHome/issues/5716
 
 <!--

internal/filtering/http.go

@@ -555,6 +555,7 @@ func (d *DNSFilter) RegisterFilteringHandlers() {
 
 	registerHTTP(http.MethodGet, "/control/rewrite/list", d.handleRewriteList)
 	registerHTTP(http.MethodPost, "/control/rewrite/add", d.handleRewriteAdd)
+	registerHTTP(http.MethodPut, "/control/rewrite/update", d.handleRewriteUpdate)
 	registerHTTP(http.MethodPost, "/control/rewrite/delete", d.handleRewriteDelete)
 
 	registerHTTP(http.MethodGet, "/control/blocked_services/services", d.handleBlockedServicesIDs)

internal/filtering/rewritehttp.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/golibs/log"
+	"golang.org/x/exp/slices"
 )
 
 // TODO(d.kolyshev): Use [rewrite.Item] instead.
@@ -91,3 +92,62 @@ func (d *DNSFilter) handleRewriteDelete(w http.ResponseWriter, r *http.Request)
 
 	d.Config.ConfigModified()
 }
+
+// rewriteUpdateJSON is a struct for JSON object with rewrite rule update info.
+type rewriteUpdateJSON struct {
+	Target rewriteEntryJSON `json:"target"`
+	Update rewriteEntryJSON `json:"update"`
+}
+
+// handleRewriteUpdate is the handler for the PUT /control/rewrite/update HTTP
+// API.
+func (d *DNSFilter) handleRewriteUpdate(w http.ResponseWriter, r *http.Request) {
+	updateJSON := rewriteUpdateJSON{}
+	err := json.NewDecoder(r.Body).Decode(&updateJSON)
+	if err != nil {
+		aghhttp.Error(r, w, http.StatusBadRequest, "json.Decode: %s", err)
+
+		return
+	}
+
+	rwDel := &LegacyRewrite{
+		Domain: updateJSON.Target.Domain,
+		Answer: updateJSON.Target.Answer,
+	}
+
+	rwAdd := &LegacyRewrite{
+		Domain: updateJSON.Update.Domain,
+		Answer: updateJSON.Update.Answer,
+	}
+
+	err = rwAdd.normalize()
+	if err != nil {
+		// Shouldn't happen currently, since normalize only returns a non-nil
+		// error when a rewrite is nil, but be change-proof.
+		aghhttp.Error(r, w, http.StatusBadRequest, "normalizing: %s", err)
+
+		return
+	}
+
+	index := -1
+	defer func() {
+		if index >= 0 {
+			d.Config.ConfigModified()
+		}
+	}()
+
+	d.confLock.Lock()
+	defer d.confLock.Unlock()
+
+	index = slices.IndexFunc(d.Config.Rewrites, rwDel.equal)
+	if index == -1 {
+		aghhttp.Error(r, w, http.StatusBadRequest, "target rule not found")
+
+		return
+	}
+
+	d.Config.Rewrites = slices.Replace(d.Config.Rewrites, index, index+1, rwAdd)
+
+	log.Debug("rewrite: removed element: %s -> %s", rwDel.Domain, rwDel.Answer)
+	log.Debug("rewrite: added element: %s -> %s", rwAdd.Domain, rwAdd.Answer)
+}

internal/filtering/rewritehttp_test.go

@@ -0,0 +1,237 @@
+package filtering_test
+
+import (
+	"bytes"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
+	"github.com/AdguardTeam/golibs/testutil"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TODO(d.kolyshev): Use [rewrite.Item] instead.
+type rewriteJSON struct {
+	Domain string `json:"domain"`
+	Answer string `json:"answer"`
+}
+
+type rewriteUpdateJSON struct {
+	Target rewriteJSON `json:"target"`
+	Update rewriteJSON `json:"update"`
+}
+
+const (
+	// testTimeout is the common timeout for tests.
+	testTimeout = 100 * time.Millisecond
+
+	listURL   = "/control/rewrite/list"
+	addURL    = "/control/rewrite/add"
+	deleteURL = "/control/rewrite/delete"
+	updateURL = "/control/rewrite/update"
+
+	decodeErrorMsg = "json.Decode: json: cannot unmarshal string into Go value of type" +
+		" filtering.rewriteEntryJSON\n"
+)
+
+func TestDNSFilter_handleRewriteHTTP(t *testing.T) {
+	confModCh := make(chan struct{})
+	reqCh := make(chan struct{})
+	testRewrites := []*rewriteJSON{
+		{Domain: "example.local", Answer: "example.rewrite"},
+		{Domain: "one.local", Answer: "one.rewrite"},
+	}
+
+	testRewritesJSON, mErr := json.Marshal(testRewrites)
+	require.NoError(t, mErr)
+
+	testCases := []struct {
+		reqData     any
+		name        string
+		url         string
+		method      string
+		wantList    []*rewriteJSON
+		wantBody    string
+		wantConfMod bool
+		wantStatus  int
+	}{{
+		name:        "list",
+		url:         listURL,
+		method:      http.MethodGet,
+		reqData:     nil,
+		wantConfMod: false,
+		wantStatus:  http.StatusOK,
+		wantBody:    string(testRewritesJSON) + "\n",
+		wantList:    testRewrites,
+	}, {
+		name:        "add",
+		url:         addURL,
+		method:      http.MethodPost,
+		reqData:     rewriteJSON{Domain: "add.local", Answer: "add.rewrite"},
+		wantConfMod: true,
+		wantStatus:  http.StatusOK,
+		wantBody:    "",
+		wantList: append(
+			testRewrites,
+			&rewriteJSON{Domain: "add.local", Answer: "add.rewrite"},
+		),
+	}, {
+		name:        "add_error",
+		url:         addURL,
+		method:      http.MethodPost,
+		reqData:     "invalid_json",
+		wantConfMod: false,
+		wantStatus:  http.StatusBadRequest,
+		wantBody:    decodeErrorMsg,
+		wantList:    testRewrites,
+	}, {
+		name:        "delete",
+		url:         deleteURL,
+		method:      http.MethodPost,
+		reqData:     rewriteJSON{Domain: "one.local", Answer: "one.rewrite"},
+		wantConfMod: true,
+		wantStatus:  http.StatusOK,
+		wantBody:    "",
+		wantList:    []*rewriteJSON{{Domain: "example.local", Answer: "example.rewrite"}},
+	}, {
+		name:        "delete_error",
+		url:         deleteURL,
+		method:      http.MethodPost,
+		reqData:     "invalid_json",
+		wantConfMod: false,
+		wantStatus:  http.StatusBadRequest,
+		wantBody:    decodeErrorMsg,
+		wantList:    testRewrites,
+	}, {
+		name:   "update",
+		url:    updateURL,
+		method: http.MethodPut,
+		reqData: rewriteUpdateJSON{
+			Target: rewriteJSON{Domain: "one.local", Answer: "one.rewrite"},
+			Update: rewriteJSON{Domain: "upd.local", Answer: "upd.rewrite"},
+		},
+		wantConfMod: true,
+		wantStatus:  http.StatusOK,
+		wantBody:    "",
+		wantList: []*rewriteJSON{
+			{Domain: "example.local", Answer: "example.rewrite"},
+			{Domain: "upd.local", Answer: "upd.rewrite"},
+		},
+	}, {
+		name:        "update_error",
+		url:         updateURL,
+		method:      http.MethodPut,
+		reqData:     "invalid_json",
+		wantConfMod: false,
+		wantStatus:  http.StatusBadRequest,
+		wantBody: "json.Decode: json: cannot unmarshal string into Go value of type" +
+			" filtering.rewriteUpdateJSON\n",
+		wantList: testRewrites,
+	}, {
+		name:   "update_error_target",
+		url:    updateURL,
+		method: http.MethodPut,
+		reqData: rewriteUpdateJSON{
+			Target: rewriteJSON{Domain: "inv.local", Answer: "inv.rewrite"},
+			Update: rewriteJSON{Domain: "upd.local", Answer: "upd.rewrite"},
+		},
+		wantConfMod: false,
+		wantStatus:  http.StatusBadRequest,
+		wantBody:    "target rule not found\n",
+		wantList:    testRewrites,
+	}}
+
+	for _, tc := range testCases {
+		onConfModified := func() {
+			if !tc.wantConfMod {
+				panic("config modified has been fired")
+			}
+
+			testutil.RequireSend(testutil.PanicT{}, confModCh, struct{}{}, testTimeout)
+		}
+
+		t.Run(tc.name, func(t *testing.T) {
+			handlers := make(map[string]http.Handler)
+
+			d, err := filtering.New(&filtering.Config{
+				ConfigModified: onConfModified,
+				HTTPRegister: func(_, url string, handler http.HandlerFunc) {
+					handlers[url] = handler
+				},
+				Rewrites: rewriteEntriesToLegacyRewrites(testRewrites),
+			}, nil)
+			require.NoError(t, err)
+			t.Cleanup(d.Close)
+
+			d.RegisterFilteringHandlers()
+			require.NotEmpty(t, handlers)
+			require.Contains(t, handlers, listURL)
+			require.Contains(t, handlers, tc.url)
+
+			var body io.Reader
+			if tc.reqData != nil {
+				data, rErr := json.Marshal(tc.reqData)
+				require.NoError(t, rErr)
+
+				body = bytes.NewReader(data)
+			}
+
+			r := httptest.NewRequest(tc.method, tc.url, body)
+			w := httptest.NewRecorder()
+
+			go func() {
+				handlers[tc.url].ServeHTTP(w, r)
+
+				testutil.RequireSend(testutil.PanicT{}, reqCh, struct{}{}, testTimeout)
+			}()
+
+			if tc.wantConfMod {
+				testutil.RequireReceive(t, confModCh, testTimeout)
+			}
+
+			testutil.RequireReceive(t, reqCh, testTimeout)
+			assert.Equal(t, tc.wantStatus, w.Code)
+
+			respBody, err := io.ReadAll(w.Body)
+			require.NoError(t, err)
+			assert.Equal(t, []byte(tc.wantBody), respBody)
+
+			assertRewritesList(t, handlers[listURL], tc.wantList)
+		})
+	}
+}
+
+// assertRewritesList checks if rewrites list equals the list received from the
+// handler by listURL.
+func assertRewritesList(t *testing.T, handler http.Handler, wantList []*rewriteJSON) {
+	t.Helper()
+
+	r := httptest.NewRequest(http.MethodGet, listURL, nil)
+	w := httptest.NewRecorder()
+
+	handler.ServeHTTP(w, r)
+	require.Equal(t, http.StatusOK, w.Code)
+
+	var actual []*rewriteJSON
+	err := json.NewDecoder(w.Body).Decode(&actual)
+	require.NoError(t, err)
+
+	assert.Equal(t, wantList, actual)
+}
+
+// rewriteEntriesToLegacyRewrites gets legacy rewrites from json entries.
+func rewriteEntriesToLegacyRewrites(entries []*rewriteJSON) (rw []*filtering.LegacyRewrite) {
+	for _, entry := range entries {
+		rw = append(rw, &filtering.LegacyRewrite{
+			Domain: entry.Domain,
+			Answer: entry.Answer,
+		})
+	}
+
+	return rw
+}

openapi/CHANGELOG.md

@@ -12,6 +12,18 @@
   `GET /control/dhcp/interfaces` HTTP APIs is now correctly set to
   `application/json` as opposed to `text/plain`.
 
+### New HTTP API 'PUT /control/rewrite/update'
+
+* The new `PUT /control/rewrite/update` HTTP API allows rewrite rule updates.
+  It accepts a JSON object with the following format:
+
+```json
+{
+  "target": {"domain":"example.com","answer":"answer-to-update"},
+  "update": {"domain":"example.com","answer":"new-answer"}
+}
+```
+
 
 
 ## v0.107.29: API changes

openapi/openapi.yaml

@@ -1061,6 +1061,17 @@
       'responses':
         '200':
           'description': 'OK.'
+  '/rewrite/update':
+    'put':
+      'tags':
+        - 'rewrite'
+      'operationId': 'rewriteUpdate'
+      'summary': 'Update a Rewrite rule'
+      'requestBody':
+        '$ref': '#/components/requestBodies/RewriteUpdate'
+      'responses':
+        '200':
+          'description': 'OK.'
   '/i18n/change_language':
     'post':
       'deprecated': true
@@ -1311,6 +1322,12 @@
           'schema':
             '$ref': '#/components/schemas/RewriteEntry'
       'required': true
+    'RewriteUpdate':
+      'content':
+        'application/json':
+          'schema':
+            '$ref': '#/components/schemas/RewriteUpdate'
+      'required': true
   'schemas':
     'ServerStatus':
       'type': 'object'
@@ -2702,6 +2719,14 @@
       'items':
         '$ref': '#/components/schemas/RewriteEntry'
       'description': 'Rewrite rules array'
+    'RewriteUpdate':
+      'type': 'object'
+      'description': 'Rewrite rule update object'
+      'properties':
+        'target':
+          '$ref': '#/components/schemas/RewriteEntry'
+        'update':
+          '$ref': '#/components/schemas/RewriteEntry'
     'RewriteEntry':
       'type': 'object'
       'description': 'Rewrite rule'