Skip to content

Commit

Permalink
Avoid duplicate request body in gRPC request object (TykTechnologies#…
Browse files Browse the repository at this point in the history
…2213)

Potential fix for TykTechnologies#2208, described [here](TykTechnologies#2208 (comment)).
Need to add tests.
  • Loading branch information
matiasinsaurralde authored and buger committed Jun 16, 2019
1 parent cead6ce commit 1a3b173
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 10 deletions.
1 change: 1 addition & 0 deletions apidef/api_definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ type MiddlewareDefinition struct {
Name string `bson:"name" json:"name"`
Path string `bson:"path" json:"path"`
RequireSession bool `bson:"require_session" json:"require_session"`
RawBodyOnly bool `bson:"raw_body_only" json:"raw_body_only"`
}

type MiddlewareIdExtractor struct {
Expand Down
8 changes: 4 additions & 4 deletions gateway/api_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ func processSpec(spec *APISpec, apisByListen map[string]int,
)
} else if mwDriver != apidef.OttoDriver {
coprocessLog.Debug("Registering coprocess middleware, hook name: ", obj.Name, "hook type: Pre", ", driver: ", mwDriver)
mwAppendEnabled(&chainArray, &CoProcessMiddleware{baseMid, coprocess.HookType_Pre, obj.Name, mwDriver})
mwAppendEnabled(&chainArray, &CoProcessMiddleware{baseMid, coprocess.HookType_Pre, obj.Name, mwDriver, obj.RawBodyOnly})
} else {
chainArray = append(chainArray, createDynamicMiddleware(obj.Name, true, obj.RequireSession, baseMid))
}
Expand Down Expand Up @@ -336,7 +336,7 @@ func processSpec(spec *APISpec, apisByListen map[string]int,
coprocessLog.Debug("Registering coprocess middleware, hook name: ", mwAuthCheckFunc.Name, "hook type: CustomKeyCheck", ", driver: ", mwDriver)

newExtractor(spec, baseMid)
mwAppendEnabled(&authArray, &CoProcessMiddleware{baseMid, coprocess.HookType_CustomKeyCheck, mwAuthCheckFunc.Name, mwDriver})
mwAppendEnabled(&authArray, &CoProcessMiddleware{baseMid, coprocess.HookType_CustomKeyCheck, mwAuthCheckFunc.Name, mwDriver, mwAuthCheckFunc.RawBodyOnly})
}

if ottoAuth {
Expand Down Expand Up @@ -375,7 +375,7 @@ func processSpec(spec *APISpec, apisByListen map[string]int,
)
} else {
coprocessLog.Debug("Registering coprocess middleware, hook name: ", obj.Name, "hook type: Pre", ", driver: ", mwDriver)
mwAppendEnabled(&chainArray, &CoProcessMiddleware{baseMid, coprocess.HookType_PostKeyAuth, obj.Name, mwDriver})
mwAppendEnabled(&chainArray, &CoProcessMiddleware{baseMid, coprocess.HookType_PostKeyAuth, obj.Name, mwDriver, obj.RawBodyOnly})
}
}

Expand Down Expand Up @@ -408,7 +408,7 @@ func processSpec(spec *APISpec, apisByListen map[string]int,
)
} else if mwDriver != apidef.OttoDriver {
coprocessLog.Debug("Registering coprocess middleware, hook name: ", obj.Name, "hook type: Post", ", driver: ", mwDriver)
mwAppendEnabled(&chainArray, &CoProcessMiddleware{baseMid, coprocess.HookType_Post, obj.Name, mwDriver})
mwAppendEnabled(&chainArray, &CoProcessMiddleware{baseMid, coprocess.HookType_Post, obj.Name, mwDriver, obj.RawBodyOnly})
} else {
chainArray = append(chainArray, createDynamicMiddleware(obj.Name, false, obj.RequireSession, baseMid))
}
Expand Down
25 changes: 19 additions & 6 deletions gateway/coprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type CoProcessMiddleware struct {
HookType coprocess.HookType
HookName string
MiddlewareDriver apidef.MiddlewareDriver
RawBodyOnly bool
}

func (mw *CoProcessMiddleware) Name() string {
Expand Down Expand Up @@ -68,7 +69,7 @@ type CoProcessor struct {
}

// ObjectFromRequest constructs a CoProcessObject from a given http.Request.
func (c *CoProcessor) ObjectFromRequest(r *http.Request) *coprocess.Object {
func (c *CoProcessor) ObjectFromRequest(r *http.Request) (*coprocess.Object, error) {
headers := ProtoMap(r.Header)

host := r.Host
Expand Down Expand Up @@ -101,8 +102,12 @@ func (c *CoProcessor) ObjectFromRequest(r *http.Request) *coprocess.Object {

if r.Body != nil {
defer r.Body.Close()
miniRequestObject.RawBody, _ = ioutil.ReadAll(r.Body)
if utf8.Valid(miniRequestObject.RawBody) {
var err error
miniRequestObject.RawBody, err = ioutil.ReadAll(r.Body)
if err != nil {
return nil, err
}
if utf8.Valid(miniRequestObject.RawBody) && !c.Middleware.RawBodyOnly {
miniRequestObject.Body = string(miniRequestObject.RawBody)
}
}
Expand All @@ -125,7 +130,11 @@ func (c *CoProcessor) ObjectFromRequest(r *http.Request) *coprocess.Object {
if c.Middleware != nil {
configDataAsJson := []byte("{}")
if len(c.Middleware.Spec.ConfigData) > 0 {
configDataAsJson, _ = json.Marshal(c.Middleware.Spec.ConfigData)
var err error
configDataAsJson, err = json.Marshal(c.Middleware.Spec.ConfigData)
if err != nil {
return nil, err
}
}

object.Spec = map[string]string{
Expand All @@ -144,7 +153,7 @@ func (c *CoProcessor) ObjectFromRequest(r *http.Request) *coprocess.Object {
}
}

return object
return object, nil
}

// ObjectPostProcess does CoProcessObject post-processing (adding/removing headers or params, etc.).
Expand Down Expand Up @@ -260,7 +269,11 @@ func (m *CoProcessMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Requ
// HookType: coprocess.PreHook,
}

object := coProcessor.ObjectFromRequest(r)
object, err := coProcessor.ObjectFromRequest(r)
if err != nil {
logger.WithError(err).Error("Failed to build request object")
return errors.New("Middleware error"), 500
}

returnObject, err := coProcessor.Dispatch(object)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions gateway/coprocess_dummy.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type CoProcessMiddleware struct {
HookType coprocess.HookType
HookName string
MiddlewareDriver apidef.MiddlewareDriver
RawBodyOnly bool
}

func (m *CoProcessMiddleware) Name() string {
Expand Down

0 comments on commit 1a3b173

Please sign in to comment.