Skip to content

fix(net/ghttp): update response message handling in MiddlewareHandlerResponse #4162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Feb 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ jobs:
strategy:
matrix:
go-version: [ 'stable' ]

name: golang-ci-lint
runs-on: ubuntu-latest
steps:
Expand All @@ -43,7 +44,7 @@ jobs:
uses: golangci/golangci-lint-action@v6
with:
# Required: specify the golangci-lint version without the patch version to always use the latest patch.
version: v1.62.2
version: v1.64.5
only-new-issues: true
skip-cache: true
github-token: ${{ secrets.GITHUB_TOKEN }}
Expand Down
8 changes: 7 additions & 1 deletion cmd/gf/internal/cmd/genpbentity/genpbentity.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type (
g.Meta `name:"pbentity" config:"{CGenPbEntityConfig}" brief:"{CGenPbEntityBrief}" eg:"{CGenPbEntityEg}" ad:"{CGenPbEntityAd}"`
Path string `name:"path" short:"p" brief:"{CGenPbEntityBriefPath}" d:"manifest/protobuf/pbentity"`
Package string `name:"package" short:"k" brief:"{CGenPbEntityBriefPackage}"`
GoPackage string `name:"goPackage" short:"g" brief:"{CGenPbEntityBriefGoPackage}"`
Link string `name:"link" short:"l" brief:"{CGenPbEntityBriefLink}"`
Tables string `name:"tables" short:"t" brief:"{CGenPbEntityBriefTables}"`
Prefix string `name:"prefix" short:"f" brief:"{CGenPbEntityBriefPrefix}"`
Expand Down Expand Up @@ -111,6 +112,7 @@ CONFIGURATION SUPPORT
`
CGenPbEntityBriefPath = `directory path for generated files storing`
CGenPbEntityBriefPackage = `package path for all entity proto files`
CGenPbEntityBriefGoPackage = `go package path for all entity proto files`
CGenPbEntityBriefLink = `database configuration, the same as the ORM configuration of GoFrame`
CGenPbEntityBriefTables = `generate models only for given tables, multiple table names separated with ','`
CGenPbEntityBriefPrefix = `add specified prefix for all entity names and entity proto files`
Expand Down Expand Up @@ -236,6 +238,7 @@ func init() {
`CGenPbEntityAd`: CGenPbEntityAd,
`CGenPbEntityBriefPath`: CGenPbEntityBriefPath,
`CGenPbEntityBriefPackage`: CGenPbEntityBriefPackage,
`CGenPbEntityBriefGoPackage`: CGenPbEntityBriefGoPackage,
`CGenPbEntityBriefLink`: CGenPbEntityBriefLink,
`CGenPbEntityBriefTables`: CGenPbEntityBriefTables,
`CGenPbEntityBriefPrefix`: CGenPbEntityBriefPrefix,
Expand Down Expand Up @@ -369,10 +372,13 @@ func generatePbEntityContentFile(ctx context.Context, in CGenPbEntityInternalInp
}
}
}
if in.GoPackage == "" {
in.GoPackage = in.Package
}
entityContent := gstr.ReplaceByMap(getTplPbEntityContent(""), g.MapStrStr{
"{Imports}": packageImportsArray.Join("\n"),
"{PackageName}": gfile.Basename(in.Package),
"{GoPackage}": in.Package,
"{GoPackage}": in.GoPackage,
"{OptionContent}": in.Option,
"{EntityMessage}": entityMessageDefine,
})
Expand Down
2 changes: 1 addition & 1 deletion contrib/config/polaris/polaris.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (c *Client) doUpdate(ctx context.Context) (err error) {
return gerror.New("config file is empty")
}
var j *gjson.Json
if j, err = gjson.DecodeToJson([]byte(c.client.GetContent())); err != nil {
if j, err = gjson.LoadContent([]byte(c.client.GetContent())); err != nil {
return gerror.Wrap(err, `parse config map item from polaris failed`)
}
c.value.Set(j)
Expand Down
2 changes: 1 addition & 1 deletion net/ghttp/ghttp_middleware_handler_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ func MiddlewareHandlerResponse(r *Request) {
msg = err.Error()
} else {
if r.Response.Status > 0 && r.Response.Status != http.StatusOK {
msg = http.StatusText(r.Response.Status)
switch r.Response.Status {
case http.StatusNotFound:
code = gcode.CodeNotFound
Expand All @@ -77,6 +76,7 @@ func MiddlewareHandlerResponse(r *Request) {
} else {
code = gcode.CodeOK
}
msg = code.Message()
}

r.Response.WriteJson(DefaultHandlerResponse{
Expand Down
20 changes: 10 additions & 10 deletions net/ghttp/ghttp_request_param.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,28 +268,28 @@ func (r *Request) parseForm() {
if r.ContentLength == 0 {
return
}

if contentType := r.Header.Get("Content-Type"); contentType != "" {
var (
err error
repeatableRead = true
)
if gstr.Contains(contentType, "multipart/") {
var isMultiPartRequest = gstr.Contains(contentType, "multipart/")
var isFormRequest = gstr.Contains(contentType, "form")
var err error

if !isMultiPartRequest {
// To avoid big memory consuming.
// The `multipart/` type form always contains binary data, which is not necessary read twice.
repeatableRead = false
r.MakeBodyRepeatableRead(true)
}
if isMultiPartRequest {
// multipart/form-data, multipart/mixed
if err = r.ParseMultipartForm(r.Server.config.FormParsingMemory); err != nil {
panic(gerror.WrapCode(gcode.CodeInvalidRequest, err, "r.ParseMultipartForm failed"))
}
} else if gstr.Contains(contentType, "form") {
} else if isFormRequest {
// application/x-www-form-urlencoded
if err = r.Request.ParseForm(); err != nil {
panic(gerror.WrapCode(gcode.CodeInvalidRequest, err, "r.Request.ParseForm failed"))
}
}
if repeatableRead {
r.MakeBodyRepeatableRead(true)
}
if len(r.PostForm) > 0 {
// Parse the form data using united parsing way.
params := ""
Expand Down
4 changes: 2 additions & 2 deletions net/ghttp/ghttp_request_param_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (r *Request) doGetRequestStruct(pointer interface{}, mapping ...map[string]
return data, nil
}
// `in` Tag Struct values.
if err = r.mergeInTagStructValue(data, pointer); err != nil {
if err = r.mergeInTagStructValue(data); err != nil {
return data, nil
}

Expand Down Expand Up @@ -239,7 +239,7 @@ func (r *Request) mergeDefaultStructValue(data map[string]interface{}, pointer i
}

// mergeInTagStructValue merges the request parameters with header or cookie values from struct `in` tag definition.
func (r *Request) mergeInTagStructValue(data map[string]interface{}, pointer interface{}) error {
func (r *Request) mergeInTagStructValue(data map[string]interface{}) error {
fields := r.serveHandler.Handler.Info.ReqStructFields
if len(fields) > 0 {
var (
Expand Down
17 changes: 0 additions & 17 deletions net/ghttp/ghttp_server_service_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,20 +282,3 @@ func createRouterFunc(funcInfo handlerFuncInfo) func(r *Request) {
}
}
}

// trimGeneric removes type definitions string from response type name if generic
func trimGeneric(structName string) string {
var (
leftBraceIndex = strings.LastIndex(structName, "[") // for generic, it is faster to start at the end than at the beginning
rightBraceIndex = strings.LastIndex(structName, "]")
)
if leftBraceIndex == -1 || rightBraceIndex == -1 {
// not found '[' or ']'
return structName
} else if leftBraceIndex+1 == rightBraceIndex {
// may be a slice, because generic is '[X]', not '[]'
// to be compatible with bad return parameter type: []XxxRes
return structName
}
return structName[:leftBraceIndex]
}
10 changes: 5 additions & 5 deletions net/ghttp/ghttp_z_unit_feature_openapi_swagger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func Test_OpenApi_Swagger(t *testing.T) {
c := g.Client()
c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)

t.Assert(gstr.Contains(c.GetContent(ctx, "/swagger/"), `API Reference`), true)
Expand Down Expand Up @@ -116,9 +116,9 @@ func Test_OpenApi_Multiple_Methods_Swagger(t *testing.T) {
c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

// Only works on GET & POST methods.
t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)
t.Assert(c.PostContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.PostContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.PostContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)

// Not works on other methods.
Expand Down Expand Up @@ -176,9 +176,9 @@ func Test_OpenApi_Method_All_Swagger(t *testing.T) {
c := g.Client()
c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)
t.Assert(c.PostContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.PostContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.PostContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)

t.Assert(gstr.Contains(c.GetContent(ctx, "/swagger/"), `API Reference`), true)
Expand Down
36 changes: 36 additions & 0 deletions net/ghttp/ghttp_z_unit_feature_request_struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,42 @@ func Test_Params_ParseForm(t *testing.T) {
})
}

// https://github.com/gogf/gf/pull/4143
func Test_Params_ParseForm_FixMakeBodyRepeatableRead(t *testing.T) {
type User struct {
Id int
Name string
}
s := g.Server(guid.S())
s.BindHandler("/parse-form", func(r *ghttp.Request) {
var user *User
if err := r.ParseForm(&user); err != nil {
r.Response.WriteExit(err)
}
hasBody := len(r.GetBody()) > 0
r.Response.WriteExit(hasBody)
})
s.SetDumpRouterMap(false)
s.Start()
defer s.Shutdown()

time.Sleep(100 * time.Millisecond)
gtest.C(t, func(t *gtest.T) {
c := g.Client()
c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))
t.Assert(c.GetContent(ctx, "/parse-form"), `false`)
t.Assert(c.GetContent(ctx, "/parse-form", g.Map{
"id": 1,
"name": "john",
}), false)
t.Assert(c.PostContent(ctx, "/parse-form"), `false`)
t.Assert(c.PostContent(ctx, "/parse-form", g.Map{
"id": 1,
"name": "john",
}), true)
})
}

func Test_Params_ComplexJsonStruct(t *testing.T) {
type ItemEnv struct {
Type string
Expand Down
48 changes: 24 additions & 24 deletions net/ghttp/ghttp_z_unit_feature_router_strict_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func Test_Router_Handler_Strict_WithObject(t *testing.T) {
client := g.Client()
client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(client.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)
})
}
Expand Down Expand Up @@ -153,8 +153,8 @@ func Test_Router_Handler_Strict_WithObjectAndMeta(t *testing.T) {
client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(client.GetContent(ctx, "/"), `{"code":65,"message":"Not Found","data":null}`)
t.Assert(client.GetContent(ctx, "/custom-test1?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/custom-test2?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/custom-test1?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/custom-test2?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.PostContent(ctx, "/custom-test2?age=18&name=john"), `{"code":65,"message":"Not Found","data":null}`)
})
}
Expand Down Expand Up @@ -184,17 +184,17 @@ func Test_Router_Handler_Strict_Group_Bind(t *testing.T) {
client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(client.GetContent(ctx, "/"), `{"code":65,"message":"Not Found","data":null}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test1?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test2?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test1?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test2?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.PostContent(ctx, "/api/v1/custom-test2?age=18&name=john"), `{"code":65,"message":"Not Found","data":null}`)

t.Assert(client.GetContent(ctx, "/api/v1/custom-test3?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test4?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test3?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test4?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)

t.Assert(client.GetContent(ctx, "/api/v2/custom-test1?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test2?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test3?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test4?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test1?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test2?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test3?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test4?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)
})
}

Expand Down Expand Up @@ -251,7 +251,7 @@ func Test_Issue1708(t *testing.T) {
`
t.Assert(
client.PostContent(ctx, "/test", content),
`{"code":0,"message":"","data":{"page":0,"size":0,"targetType":"topic","targetId":10785,"test":[[{"name":"123"}]]}}`,
`{"code":0,"message":"OK","data":{"page":0,"size":0,"targetType":"topic","targetId":10785,"test":[[{"name":"123"}]]}}`,
)
})
}
Expand Down Expand Up @@ -295,7 +295,7 @@ func Test_Custom_Slice_Type_Attribute(t *testing.T) {
`
t.Assert(
client.PostContent(ctx, "/test", content),
`{"code":0,"message":"","data":{"Content":"{\"Id\":1,\"List\":{\"key\":[\"a\",\"b\",\"c\"]}}"}}`,
`{"code":0,"message":"OK","data":{"Content":"{\"Id\":1,\"List\":{\"key\":[\"a\",\"b\",\"c\"]}}"}}`,
)
})
}
Expand Down Expand Up @@ -370,12 +370,12 @@ func Test_Router_Handler_Strict_WithGeneric(t *testing.T) {
client := g.Client()
client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(client.GetContent(ctx, "/test1?age=1"), `{"code":0,"message":"","data":{"Age":{"Test":1}}}`)
t.Assert(client.GetContent(ctx, "/test1_slice?age=1"), `{"code":0,"message":"","data":[{"Age":{"Test":1}}]}`)
t.Assert(client.GetContent(ctx, "/test2?age=2"), `{"code":0,"message":"","data":{"Test":2}}`)
t.Assert(client.GetContent(ctx, "/test2_slice?age=2"), `{"code":0,"message":"","data":[{"Test":2}]}`)
t.Assert(client.GetContent(ctx, "/test3?age=3"), `{"code":0,"message":"","data":{"Test":3}}`)
t.Assert(client.GetContent(ctx, "/test3_slice?age=3"), `{"code":0,"message":"","data":[{"Test":3}]}`)
t.Assert(client.GetContent(ctx, "/test1?age=1"), `{"code":0,"message":"OK","data":{"Age":{"Test":1}}}`)
t.Assert(client.GetContent(ctx, "/test1_slice?age=1"), `{"code":0,"message":"OK","data":[{"Age":{"Test":1}}]}`)
t.Assert(client.GetContent(ctx, "/test2?age=2"), `{"code":0,"message":"OK","data":{"Test":2}}`)
t.Assert(client.GetContent(ctx, "/test2_slice?age=2"), `{"code":0,"message":"OK","data":[{"Test":2}]}`)
t.Assert(client.GetContent(ctx, "/test3?age=3"), `{"code":0,"message":"OK","data":{"Test":3}}`)
t.Assert(client.GetContent(ctx, "/test3_slice?age=3"), `{"code":0,"message":"OK","data":[{"Test":3}]}`)
})
}

Expand Down Expand Up @@ -415,7 +415,7 @@ func Test_Router_Handler_Strict_ParameterCaseSensitive(t *testing.T) {
for i := 0; i < 1000; i++ {
t.Assert(
client.PostContent(ctx, "/api/111", `{"Path":"222"}`),
`{"code":0,"message":"","data":{"Path":"222"}}`,
`{"code":0,"message":"OK","data":{"Path":"222"}}`,
)
}
})
Expand Down Expand Up @@ -474,10 +474,10 @@ func Test_JsonRawMessage_Issue3449(t *testing.T) {
},
}

expect1 := `{"code":0,"message":"","data":{"name":"test","jsonRaw":[{"jkey1":"11","jkey2":"12"},{"jkey1":"21","jkey2":"22"}]}}`
expect1 := `{"code":0,"message":"OK","data":{"name":"test","jsonRaw":[{"jkey1":"11","jkey2":"12"},{"jkey1":"21","jkey2":"22"}]}}`
t.Assert(client.PostContent(ctx, "/test", data), expect1)

expect2 := `{"code":0,"message":"","data":{"name":"test","jsonRaw":{"jkey1":"11","jkey2":"12"}}}`
expect2 := `{"code":0,"message":"OK","data":{"name":"test","jsonRaw":{"jkey1":"11","jkey2":"12"}}}`
t.Assert(client.PostContent(ctx, "/test", map[string]any{
"Name": "test",
"jsonRaw": v1,
Expand Down Expand Up @@ -524,13 +524,13 @@ func Test_NullString_Issue3465(t *testing.T) {
"name": "null",
}

expect1 := `{"code":0,"message":"","data":{"name":["null"]}}`
expect1 := `{"code":0,"message":"OK","data":{"name":["null"]}}`
t.Assert(client.GetContent(ctx, "/test", data1), expect1)

data2 := map[string]any{
"name": []string{"null", "null"},
}
expect2 := `{"code":0,"message":"","data":{"name":["null","null"]}}`
expect2 := `{"code":0,"message":"OK","data":{"name":["null","null"]}}`
t.Assert(client.GetContent(ctx, "/test", data2), expect2)

})
Expand Down
Loading
Loading