From 7d1ca48ae49689766dd294e32ad6db3b2deaa52f Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 15 Feb 2021 13:56:41 +0300 Subject: [PATCH] Pull request #1005: home: imp large req handling Merge in DNS/adguard-home from 2675-larger-requests to master Updates #2675. Squashed commit of the following: commit 2b45c9bfdc817980204b11de768b425fb72a6488 Author: Ainar Garipov Date: Mon Feb 15 13:38:44 2021 +0300 home: imp names commit dad39ae7ee35346ea91f15665acc93ba0b5653df Author: Ainar Garipov Date: Mon Feb 15 13:31:53 2021 +0300 home: imp large req handling --- CHANGELOG.md | 5 ++-- internal/home/middlewares.go | 38 +++++++++++++++++++++++-------- internal/home/middlewares_test.go | 6 ++--- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a7407c1..775ebcc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,8 +23,8 @@ and this project adheres to longer prevent the DHCP server from starting ([#2667]). - The server name sent by clients of TLS APIs is not only checked when `strict_sni_check` is enabled ([#2664]). -- HTTP API request body size limit for the `POST /control/access/set` HTTP API - is increased ([#2666]). +- HTTP API request body size limit for the `POST /control/access/set` and `POST + /control/filtering/set_rules` HTTP APIs is increased ([#2666], [#2675]). ### Fixed @@ -45,6 +45,7 @@ and this project adheres to [#2664]: https://github.com/AdguardTeam/AdGuardHome/issues/2664 [#2666]: https://github.com/AdguardTeam/AdGuardHome/issues/2666 [#2667]: https://github.com/AdguardTeam/AdGuardHome/issues/2667 +[#2675]: https://github.com/AdguardTeam/AdGuardHome/issues/2675 [#2678]: https://github.com/AdguardTeam/AdGuardHome/issues/2678 diff --git a/internal/home/middlewares.go b/internal/home/middlewares.go index 7d34f73e..de38c3fa 100644 --- a/internal/home/middlewares.go +++ b/internal/home/middlewares.go @@ -22,8 +22,30 @@ func withMiddlewares(h http.Handler, middlewares ...middleware) (wrapped http.Ha return wrapped } -// RequestBodySizeLimit is maximum request body length in bytes. -const RequestBodySizeLimit = 64 * 1024 +// defaultReqBodySzLim is the default maximum request body size. +const defaultReqBodySzLim = 64 * 1024 + +// largerReqBodySzLim is the maximum request body size for APIs expecting larger +// requests. +const largerReqBodySzLim = 4 * 1024 * 1024 + +// expectsLargerRequests shows if this request should use a larger body size +// limit. These are exceptions for poorly designed current APIs as well as APIs +// that are designed to expect large files and requests. Remove once the new, +// better APIs are up. +// +// See https://github.com/AdguardTeam/AdGuardHome/issues/2666 and +// https://github.com/AdguardTeam/AdGuardHome/issues/2675. +func expectsLargerRequests(r *http.Request) (ok bool) { + m := r.Method + if m != http.MethodPost { + return false + } + + p := r.URL.Path + return p == "/control/access/set" || + p == "/control/filtering/set_rules" +} // limitRequestBody wraps underlying handler h, making it's request's body Read // method limited. @@ -31,16 +53,12 @@ func limitRequestBody(h http.Handler) (limited http.Handler) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var err error - var bodySizeLimit int64 = RequestBodySizeLimit - if u := r.URL; u.Path == "/control/access/set" { - // An exception for a poorly designed API. Remove once - // the new, better API is up. - // - // See https://github.com/AdguardTeam/AdGuardHome/issues/2666. - bodySizeLimit *= 4 + var szLim int64 = defaultReqBodySzLim + if expectsLargerRequests(r) { + szLim = largerReqBodySzLim } - r.Body, err = aghio.LimitReadCloser(r.Body, bodySizeLimit) + r.Body, err = aghio.LimitReadCloser(r.Body, szLim) if err != nil { log.Error("limitRequestBody: %s", err) diff --git a/internal/home/middlewares_test.go b/internal/home/middlewares_test.go index fbd7a214..8397302b 100644 --- a/internal/home/middlewares_test.go +++ b/internal/home/middlewares_test.go @@ -14,7 +14,7 @@ import ( func TestLimitRequestBody(t *testing.T) { errReqLimitReached := &aghio.LimitReachedError{ - Limit: RequestBodySizeLimit, + Limit: defaultReqBodySzLim, } testCases := []struct { @@ -29,8 +29,8 @@ func TestLimitRequestBody(t *testing.T) { wantErr: nil, }, { name: "so_big", - body: string(make([]byte, RequestBodySizeLimit+1)), - want: make([]byte, RequestBodySizeLimit), + body: string(make([]byte, defaultReqBodySzLim+1)), + want: make([]byte, defaultReqBodySzLim), wantErr: errReqLimitReached, }, { name: "empty",