[ARVADOS] created: 2.1.0-675-g322a23b02
Git user
git at public.arvados.org
Tue Apr 20 03:11:00 UTC 2021
at 322a23b02d649e4a81ec9ae2a0de2b4398da9efc (commit)
commit 322a23b02d649e4a81ec9ae2a0de2b4398da9efc
Author: Tom Clegg <tom at curii.com>
Date: Mon Apr 19 18:04:36 2021 -0400
17566: Don't bypass controller in SDK integration tests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/sdk/go/arvadosclient/arvadosclient_test.go b/sdk/go/arvadosclient/arvadosclient_test.go
index 07ad93d11..9d6e4fe7e 100644
--- a/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@ -30,14 +30,12 @@ var _ = Suite(&MockArvadosServerSuite{})
type ServerRequiredSuite struct{}
func (s *ServerRequiredSuite) SetUpSuite(c *C) {
- arvadostest.StartAPI()
arvadostest.StartKeep(2, false)
RetryDelay = 0
}
func (s *ServerRequiredSuite) TearDownSuite(c *C) {
arvadostest.StopKeep(2)
- arvadostest.StopAPI()
}
func (s *ServerRequiredSuite) SetUpTest(c *C) {
commit 55875077de77b317ee251ed6450e14e1bfe70097
Author: Tom Clegg <tom at curii.com>
Date: Mon Apr 19 18:04:14 2021 -0400
17566: Allow large request bodies in integration tests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/sdk/python/tests/nginx.conf b/sdk/python/tests/nginx.conf
index a4336049f..35b780071 100644
--- a/sdk/python/tests/nginx.conf
+++ b/sdk/python/tests/nginx.conf
@@ -24,6 +24,7 @@ http {
server_name controller ~.*;
ssl_certificate "{{SSLCERT}}";
ssl_certificate_key "{{SSLKEY}}";
+ client_max_body_size 0;
location / {
proxy_pass http://controller;
proxy_set_header Host $http_host;
commit 89d025b0b20596b3cc79f1e8c588cbe6a5a02d85
Author: Tom Clegg <tom at curii.com>
Date: Mon Apr 19 18:01:39 2021 -0400
17566: Use configured MaxRequestSize.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 578bfc7d2..a35d00301 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -92,7 +92,10 @@ func (h *Handler) setup() {
})
oidcAuthorizer := localdb.OIDCAccessTokenAuthorizer(h.Cluster, h.db)
- rtr := router.New(federation.New(h.Cluster), api.ComposeWrappers(ctrlctx.WrapCallsInTransactions(h.db), oidcAuthorizer.WrapCalls))
+ rtr := router.New(federation.New(h.Cluster), router.Config{
+ MaxRequestSize: h.Cluster.API.MaxRequestSize,
+ WrapCalls: api.ComposeWrappers(ctrlctx.WrapCallsInTransactions(h.db), oidcAuthorizer.WrapCalls),
+ })
mux.Handle("/arvados/v1/config", rtr)
mux.Handle("/"+arvados.EndpointUserAuthenticate.Path, rtr) // must come before .../users/
mux.Handle("/arvados/v1/collections", rtr)
diff --git a/lib/controller/router/request.go b/lib/controller/router/request.go
index eae9e0a8c..06141b103 100644
--- a/lib/controller/router/request.go
+++ b/lib/controller/router/request.go
@@ -63,7 +63,11 @@ func guessAndParse(k, v string) (interface{}, error) {
func (rtr *router) loadRequestParams(req *http.Request, attrsKey string) (map[string]interface{}, error) {
err := req.ParseForm()
if err != nil {
- return nil, httpError(http.StatusBadRequest, err)
+ if err.Error() == "http: request body too large" {
+ return nil, httpError(http.StatusRequestEntityTooLarge, err)
+ } else {
+ return nil, httpError(http.StatusBadRequest, err)
+ }
}
params := map[string]interface{}{}
diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go
index a313ebc8b..5ceabbfb1 100644
--- a/lib/controller/router/router.go
+++ b/lib/controller/router/router.go
@@ -7,6 +7,7 @@ package router
import (
"context"
"fmt"
+ "math"
"net/http"
"strings"
@@ -20,24 +21,32 @@ import (
)
type router struct {
- mux *mux.Router
- backend arvados.API
- wrapCalls func(api.RoutableFunc) api.RoutableFunc
+ mux *mux.Router
+ backend arvados.API
+ config Config
+}
+
+type Config struct {
+ // Return an error if request body exceeds this size. 0 means
+ // unlimited.
+ MaxRequestSize int
+
+ // If wrapCalls is not nil, it is called once for each API
+ // method, and the returned method is used in its place. This
+ // can be used to install hooks before and after each API call
+ // and alter responses; see localdb.WrapCallsInTransaction for
+ // an example.
+ WrapCalls func(api.RoutableFunc) api.RoutableFunc
}
// New returns a new router (which implements the http.Handler
// interface) that serves requests by calling Arvados API methods on
// the given backend.
-//
-// If wrapCalls is not nil, it is called once for each API method, and
-// the returned method is used in its place. This can be used to
-// install hooks before and after each API call and alter responses;
-// see localdb.WrapCallsInTransaction for an example.
-func New(backend arvados.API, wrapCalls func(api.RoutableFunc) api.RoutableFunc) *router {
+func New(backend arvados.API, config Config) *router {
rtr := &router{
- mux: mux.NewRouter(),
- backend: backend,
- wrapCalls: wrapCalls,
+ mux: mux.NewRouter(),
+ backend: backend,
+ config: config,
}
rtr.addRoutes()
return rtr
@@ -433,8 +442,8 @@ func (rtr *router) addRoutes() {
},
} {
exec := route.exec
- if rtr.wrapCalls != nil {
- exec = rtr.wrapCalls(exec)
+ if rtr.config.WrapCalls != nil {
+ exec = rtr.config.WrapCalls(exec)
}
rtr.addRoute(route.endpoint, route.defaultOpts, exec)
}
@@ -524,8 +533,26 @@ func (rtr *router) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if r.Method == "OPTIONS" {
return
}
+ if r.Body != nil {
+ // Wrap r.Body in a http.MaxBytesReader(), otherwise
+ // r.ParseForm() uses a default max request body size
+ // of 10 megabytes. Note we rely on the Nginx
+ // configuration to enforce the real max body size.
+ max := int64(rtr.config.MaxRequestSize)
+ if max < 1 {
+ max = math.MaxInt64 - 1
+ }
+ r.Body = http.MaxBytesReader(w, r.Body, max)
+ }
if r.Method == "POST" {
- r.ParseForm()
+ err := r.ParseForm()
+ if err != nil {
+ if err.Error() == "http: request body too large" {
+ err = httpError(http.StatusRequestEntityTooLarge, err)
+ }
+ rtr.sendError(w, err)
+ return
+ }
if m := r.FormValue("_method"); m != "" {
r2 := *r
r = &r2
diff --git a/lib/controller/router/router_test.go b/lib/controller/router/router_test.go
index 18fff7c9c..0330ec425 100644
--- a/lib/controller/router/router_test.go
+++ b/lib/controller/router/router_test.go
@@ -169,7 +169,7 @@ func (s *RouterIntegrationSuite) SetUpTest(c *check.C) {
cluster.TLS.Insecure = true
arvadostest.SetServiceURL(&cluster.Services.RailsAPI, "https://"+os.Getenv("ARVADOS_TEST_API_HOST"))
url, _ := url.Parse("https://" + os.Getenv("ARVADOS_TEST_API_HOST"))
- s.rtr = New(rpc.NewConn("zzzzz", url, true, rpc.PassthroughTokenProvider), nil)
+ s.rtr = New(rpc.NewConn("zzzzz", url, true, rpc.PassthroughTokenProvider), Config{})
}
func (s *RouterIntegrationSuite) TearDownSuite(c *check.C) {
@@ -226,6 +226,34 @@ func (s *RouterIntegrationSuite) TestCollectionResponses(c *check.C) {
c.Check(jresp["kind"], check.Equals, "arvados#collection")
}
+func (s *RouterIntegrationSuite) TestMaxRequestSize(c *check.C) {
+ token := arvadostest.ActiveTokenV2
+ for _, maxRequestSize := range []int{
+ // Ensure 5M limit is enforced.
+ 5000000,
+ // Ensure 50M limit is enforced, and that a >25M body
+ // is accepted even though the default Go request size
+ // limit is 10M.
+ 50000000,
+ } {
+ s.rtr.config.MaxRequestSize = maxRequestSize
+ okstr := "a"
+ for len(okstr) < maxRequestSize/2 {
+ okstr = okstr + okstr
+ }
+
+ hdr := http.Header{"Content-Type": {"application/x-www-form-urlencoded"}}
+
+ body := bytes.NewBufferString(url.Values{"foo_bar": {okstr}}.Encode())
+ _, rr, _ := doRequest(c, s.rtr, token, "POST", `/arvados/v1/collections`, hdr, body)
+ c.Check(rr.Code, check.Equals, http.StatusOK)
+
+ body = bytes.NewBufferString(url.Values{"foo_bar": {okstr + okstr}}.Encode())
+ _, rr, _ = doRequest(c, s.rtr, token, "POST", `/arvados/v1/collections`, hdr, body)
+ c.Check(rr.Code, check.Equals, http.StatusRequestEntityTooLarge)
+ }
+}
+
func (s *RouterIntegrationSuite) TestContainerList(c *check.C) {
token := arvadostest.ActiveTokenV2
diff --git a/sdk/go/arvadosclient/arvadosclient_test.go b/sdk/go/arvadosclient/arvadosclient_test.go
index fc686ad63..07ad93d11 100644
--- a/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@ -10,7 +10,9 @@ import (
"net/http"
"os"
"testing"
+ "time"
+ "git.arvados.org/arvados.git/sdk/go/arvados"
"git.arvados.org/arvados.git/sdk/go/arvadostest"
. "gopkg.in/check.v1"
)
@@ -158,6 +160,32 @@ func (s *ServerRequiredSuite) TestAPIDiscovery_Get_noSuchParameter(c *C) {
c.Assert(value, IsNil)
}
+func (s *ServerRequiredSuite) TestCreateLarge(c *C) {
+ arv, err := MakeArvadosClient()
+ c.Assert(err, IsNil)
+
+ txt := arvados.SignLocator("d41d8cd98f00b204e9800998ecf8427e+0", arv.ApiToken, time.Now().Add(time.Minute), time.Minute, []byte(arvadostest.SystemRootToken))
+ // Ensure our request body is bigger than the Go http server's
+ // default max size, 10 MB.
+ for len(txt) < 12000000 {
+ txt = txt + " " + txt
+ }
+ txt = ". " + txt + " 0:0:foo\n"
+
+ resp := Dict{}
+ err = arv.Create("collections", Dict{
+ "ensure_unique_name": true,
+ "collection": Dict{
+ "is_trashed": true,
+ "name": "test",
+ "manifest_text": txt,
+ },
+ }, &resp)
+ c.Check(err, IsNil)
+ c.Check(resp["portable_data_hash"], Not(Equals), "")
+ c.Check(resp["portable_data_hash"], Not(Equals), "d41d8cd98f00b204e9800998ecf8427e+0")
+}
+
type UnitSuite struct{}
func (s *UnitSuite) TestUUIDMatch(c *C) {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list