This is an automated email from the ASF dual-hosted git repository.
chenjunxu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-dashboard.git
The following commit(s) were added to refs/heads/master by this push:
new 678b94d fix: error check to ensure path id doesn't conflict body id
(#1067)
678b94d is described below
commit 678b94d643c4f0bbbbcebbfe51025ada768b6165
Author: Jinchi Zhou <[email protected]>
AuthorDate: Fri Dec 25 22:58:34 2020 -0800
fix: error check to ensure path id doesn't conflict body id (#1067)
Co-authored-by: nic-chen <[email protected]>
---
api/internal/core/store/store.go | 1 -
api/internal/handler/handler.go | 21 +++++-
api/internal/handler/handler_test.go | 86 ++++++++++++++++++++++++
api/internal/handler/route/route.go | 12 ++--
api/internal/handler/route/route_test.go | 90 +++++++++++++++++++++++++-
api/internal/handler/service/service.go | 6 ++
api/internal/handler/ssl/ssl.go | 6 ++
api/internal/handler/upstream/upstream.go | 8 +++
api/internal/handler/upstream/upstream_test.go | 2 +-
api/test/e2e/id_compatible_test.go | 2 -
10 files changed, 222 insertions(+), 12 deletions(-)
diff --git a/api/internal/core/store/store.go b/api/internal/core/store/store.go
index bf00dcd..9a4b113 100644
--- a/api/internal/core/store/store.go
+++ b/api/internal/core/store/store.go
@@ -327,7 +327,6 @@ func (s *GenericStore) StringToObjPtr(str, key string)
(interface{}, error) {
objPtr := reflect.New(s.opt.ObjType)
ret := objPtr.Interface()
err := json.Unmarshal([]byte(str), ret)
- fmt.Println("ret:", ret, "s.opt.ObjType", s.opt.ObjType)
if err != nil {
log.Errorf("json marshal failed: %s", err)
return nil, fmt.Errorf("json unmarshal failed: %s", err)
diff --git a/api/internal/handler/handler.go b/api/internal/handler/handler.go
index 39256d9..fecd0d3 100644
--- a/api/internal/handler/handler.go
+++ b/api/internal/handler/handler.go
@@ -35,13 +35,16 @@
package handler
import (
- "github.com/shiningrush/droplet"
- "github.com/shiningrush/droplet/middleware"
+ "fmt"
"net/http"
"strings"
"github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
"github.com/shiningrush/droplet/data"
+ "github.com/shiningrush/droplet/middleware"
+
+ "github.com/apisix/manager-api/internal/utils"
)
type RegisterFactory func() (RouteRegister, error)
@@ -87,3 +90,17 @@ func (mw *ErrorTransformMiddleware) Handle(ctx
droplet.Context) error {
}
return nil
}
+
+func IDCompare(idOnPath string, idOnBody interface{}) error {
+ idOnBodyStr, ok := idOnBody.(string)
+ if !ok {
+ idOnBodyStr = utils.InterfaceToString(idOnBody)
+ }
+
+ // check if id on path is == to id on body ONLY if both ids are valid
+ if idOnPath != "" && idOnBodyStr != "" && idOnBodyStr != idOnPath {
+ return fmt.Errorf("ID on path (%s) doesn't match ID on body
(%s)", idOnPath, idOnBodyStr)
+ }
+
+ return nil
+}
diff --git a/api/internal/handler/handler_test.go
b/api/internal/handler/handler_test.go
new file mode 100644
index 0000000..7fd288c
--- /dev/null
+++ b/api/internal/handler/handler_test.go
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package handler
+
+import (
+ "errors"
+ "net/http"
+ "testing"
+
+ "github.com/go-playground/assert/v2"
+ "github.com/shiningrush/droplet/data"
+)
+
+func TestSpecCodeResponse(t *testing.T) {
+ err := errors.New("schema validate failed: remote_addr: Must validate
at least one schema (anyOf)")
+ resp := SpecCodeResponse(err)
+ assert.Equal(t, &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, resp)
+
+ err = errors.New("data not found")
+ resp = SpecCodeResponse(err)
+ assert.Equal(t, &data.SpecCodeResponse{StatusCode:
http.StatusNotFound}, resp)
+
+ err = errors.New("system error")
+ resp = SpecCodeResponse(err)
+ assert.Equal(t, &data.SpecCodeResponse{StatusCode:
http.StatusInternalServerError}, resp)
+}
+
+func TestIDCompare(t *testing.T) {
+ // init
+ cases := []struct {
+ idOnPath, desc string
+ idOnBody interface{}
+ wantError error
+ }{
+ {
+ desc: "ID on body is int, and it could be
considered the same as ID on path",
+ idOnPath: "1",
+ idOnBody: 1,
+ },
+ {
+ desc: "ID on body is int, and it is different from
ID on path",
+ idOnPath: "1",
+ idOnBody: 2,
+ wantError: errors.New("ID on path (1) doesn't match ID
on body (2)"),
+ },
+ {
+ desc: "ID on body is same as ID on path",
+ idOnPath: "1",
+ idOnBody: "1",
+ },
+ {
+ desc: "ID on body is different from ID on path",
+ idOnPath: "a",
+ idOnBody: "b",
+ wantError: errors.New("ID on path (a) doesn't match ID
on body (b)"),
+ },
+ {
+ desc: "No ID on body",
+ idOnPath: "1",
+ },
+ {
+ desc: "No ID on path",
+ idOnBody: 1,
+ },
+ }
+ for _, c := range cases {
+ t.Run(c.desc, func(t *testing.T) {
+ err := IDCompare(c.idOnPath, c.idOnBody)
+ assert.Equal(t, c.wantError, err)
+ })
+ }
+}
diff --git a/api/internal/handler/route/route.go
b/api/internal/handler/route/route.go
index 275ab1e..59dc6ce 100644
--- a/api/internal/handler/route/route.go
+++ b/api/internal/handler/route/route.go
@@ -347,13 +347,15 @@ type UpdateInput struct {
func (h *Handler) Update(c droplet.Context) (interface{}, error) {
input := c.Input().(*UpdateInput)
- if input.ID != "" {
- input.Route.ID = input.ID
+
+ // check if ID in body is equal ID in path
+ if err := handler.IDCompare(input.ID, input.Route.ID); err != nil {
+ return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, err
}
- if input.Route.Host != "" && len(input.Route.Hosts) > 0 {
- return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest},
- fmt.Errorf("only one of host or hosts is allowed")
+ // if has id in path, use it
+ if input.ID != "" {
+ input.Route.ID = input.ID
}
//check depend
diff --git a/api/internal/handler/route/route_test.go
b/api/internal/handler/route/route_test.go
index 0fd67c6..f84fc90 100644
--- a/api/internal/handler/route/route_test.go
+++ b/api/internal/handler/route/route_test.go
@@ -757,6 +757,86 @@ func TestRoute(t *testing.T) {
//sleep
time.Sleep(time.Duration(100) * time.Millisecond)
+ // check ID discrepancy on Update
+
+ // Fail: test the string body id value != string route id value
+ errRoute := &UpdateInput{}
+ errRoute.ID = "2"
+ err = json.Unmarshal([]byte(reqBody), errRoute)
+ assert.Nil(t, err)
+ ctx.SetInput(errRoute)
+ ret, err = handler.Update(ctx)
+ assert.NotNil(t, err)
+ assert.EqualError(t, err, "ID on path (2) doesn't match ID on body (1)")
+ assert.Equal(t, http.StatusBadRequest,
ret.(*data.SpecCodeResponse).StatusCode)
+
+ // Fail: tests the float body id value != string route id value
+ reqBodyErr := `{
+ "id": 1,
+ "uri": "/index.html",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "www.a.com",
+ "port": 80,
+ "weight": 1
+ }]
+ }
+ }`
+ errRoute = &UpdateInput{}
+ errRoute.ID = "2"
+ err = json.Unmarshal([]byte(reqBodyErr), errRoute)
+ assert.Nil(t, err)
+ ctx.SetInput(errRoute)
+ ret, err = handler.Update(ctx)
+ assert.NotNil(t, err)
+ assert.EqualError(t, err, "ID on path (2) doesn't match ID on body (1)")
+ assert.Equal(t, http.StatusBadRequest,
ret.(*data.SpecCodeResponse).StatusCode)
+
+ // Success: tests the float body id value is == string route id value
+ reqBodyErr = `{
+ "id": 10,
+ "uri": "/index.html",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "www.a.com",
+ "port": 80,
+ "weight": 1
+ }]
+ }
+ }`
+ errRoute = &UpdateInput{}
+ errRoute.ID = "10"
+ err = json.Unmarshal([]byte(reqBodyErr), errRoute)
+ assert.Nil(t, err)
+ ctx.SetInput(errRoute)
+ ret, err = handler.Update(ctx)
+ assert.Nil(t, err)
+
+ // Success: tests the Body ID can be nil
+ reqBodyErr = `{
+ "uri": "/index.html",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "www.a.com",
+ "port": 80,
+ "weight": 1
+ }]
+ }
+ }`
+ errRoute = &UpdateInput{}
+ errRoute.ID = "r1"
+ err = json.Unmarshal([]byte(reqBodyErr), errRoute)
+ assert.Nil(t, err)
+ ctx.SetInput(errRoute)
+ ret, err = handler.Update(ctx)
+ assert.Nil(t, err)
+
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
+
//list
listInput := &ListInput{}
reqBody = `{"page_size": 1, "page": 1}`
@@ -1066,7 +1146,6 @@ func TestRoute(t *testing.T) {
ctx.SetInput(inputDel)
_, err = handler.BatchDelete(ctx)
assert.Nil(t, err)
-
}
func Test_Route_With_Script(t *testing.T) {
@@ -1211,4 +1290,13 @@ func Test_Route_With_Script(t *testing.T) {
assert.Nil(t, err)
assert.Equal(t, stored.ID, route.ID)
assert.Nil(t, stored.Script)
+
+ //delete test data
+ inputDel := &BatchDelete{}
+ reqBody = `{"ids": "1"}`
+ err = json.Unmarshal([]byte(reqBody), inputDel)
+ assert.Nil(t, err)
+ ctx.SetInput(inputDel)
+ _, err = handler.BatchDelete(ctx)
+ assert.Nil(t, err)
}
diff --git a/api/internal/handler/service/service.go
b/api/internal/handler/service/service.go
index 1adb0a4..799e58a 100644
--- a/api/internal/handler/service/service.go
+++ b/api/internal/handler/service/service.go
@@ -179,6 +179,12 @@ type UpdateInput struct {
func (h *Handler) Update(c droplet.Context) (interface{}, error) {
input := c.Input().(*UpdateInput)
+
+ // check if ID in body is equal ID in path
+ if err := handler.IDCompare(input.ID, input.Service.ID); err != nil {
+ return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, err
+ }
+
if input.ID != "" {
input.Service.ID = input.ID
}
diff --git a/api/internal/handler/ssl/ssl.go b/api/internal/handler/ssl/ssl.go
index 28408cc..0c9a3f2 100644
--- a/api/internal/handler/ssl/ssl.go
+++ b/api/internal/handler/ssl/ssl.go
@@ -203,6 +203,12 @@ type UpdateInput struct {
func (h *Handler) Update(c droplet.Context) (interface{}, error) {
input := c.Input().(*UpdateInput)
+
+ // check if ID in body is equal ID in path
+ if err := handler.IDCompare(input.ID, input.SSL.ID); err != nil {
+ return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, err
+ }
+
ssl, err := ParseCert(input.Cert, input.Key)
if err != nil {
return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, err
diff --git a/api/internal/handler/upstream/upstream.go
b/api/internal/handler/upstream/upstream.go
index 672906e..790fbf0 100644
--- a/api/internal/handler/upstream/upstream.go
+++ b/api/internal/handler/upstream/upstream.go
@@ -17,12 +17,14 @@
package upstream
import (
+ "net/http"
"reflect"
"strings"
"github.com/api7/go-jsonpatch"
"github.com/gin-gonic/gin"
"github.com/shiningrush/droplet"
+ "github.com/shiningrush/droplet/data"
"github.com/shiningrush/droplet/wrapper"
wgin "github.com/shiningrush/droplet/wrapper/gin"
@@ -161,6 +163,12 @@ type UpdateInput struct {
func (h *Handler) Update(c droplet.Context) (interface{}, error) {
input := c.Input().(*UpdateInput)
+
+ // check if ID in body is equal ID in path
+ if err := handler.IDCompare(input.ID, input.Upstream.ID); err != nil {
+ return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, err
+ }
+
if input.ID != "" {
input.Upstream.ID = input.ID
}
diff --git a/api/internal/handler/upstream/upstream_test.go
b/api/internal/handler/upstream/upstream_test.go
index e72b14f..6216633 100644
--- a/api/internal/handler/upstream/upstream_test.go
+++ b/api/internal/handler/upstream/upstream_test.go
@@ -115,7 +115,7 @@ func TestUpstream(t *testing.T) {
upstream2 := &UpdateInput{}
upstream2.ID = "1"
reqBody = `{
- "id": "aaa",
+ "id": "1",
"name": "upstream3",
"description": "upstream upstream",
"type": "roundrobin",
diff --git a/api/test/e2e/id_compatible_test.go
b/api/test/e2e/id_compatible_test.go
index 90c3322..33dc865 100644
--- a/api/test/e2e/id_compatible_test.go
+++ b/api/test/e2e/id_compatible_test.go
@@ -17,7 +17,6 @@
package e2e
import (
- "fmt"
"io/ioutil"
"net/http"
"testing"
@@ -458,7 +457,6 @@ func TestID_Not_In_Body(t *testing.T) {
assert.Nil(t, err)
defer resp.Body.Close()
respBody, _ := ioutil.ReadAll(resp.Body)
- fmt.Println("string(respBody)", string(respBody))
list := gjson.Get(string(respBody), "data.rows").Value().([]interface{})
for _, item := range list {
route := item.(map[string]interface{})
