Skip to content
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

[BUG] mvc can not get query param #1566

Closed
ghost opened this issue Jul 22, 2020 · 6 comments
Closed

[BUG] mvc can not get query param #1566

ghost opened this issue Jul 22, 2020 · 6 comments

Comments

@ghost
Copy link

ghost commented Jul 22, 2020

Describe the bug
route can not handle query param when using mvc controller

To Reproduce
Steps to reproduce the behavior:
1.

mvc.Configure(app.Party("/user"), setup, func(app *mvc.Application) {
	app.Handle(new(controller.UserController))
})

type UserController struct {}

func (c *UserController) Get(req struct {
	Start int `url:"start"`
	Limit int `url:"limit"`
}) error {
	return  nil
}
GET: /usercontroller (./route\route.go:39)
     • iris-contrib/middleware/cors.(*Cors).Serve-fm (C:/Users/admin/go/pkg/mod/github.com/iris-contrib/middleware/[email protected]/cors.go:165)
     • controller.UserController.Get (./controller/user.go:67)
  1. visit url like http://localhost/user?start=1&limit=2
Result: schema: invalid path "start" (and 1 other error)
@ghost ghost added the 🐞 type:bug label Jul 22, 2020
@ghost ghost assigned kataras Jul 22, 2020
@kataras
Copy link
Owner

kataras commented Jul 24, 2020

I dont understand @ghost-void, the example runs here:

image

package main

import (
	"fmt"

	"github.com/kataras/iris/v12"
	"github.com/kataras/iris/v12/mvc"
)

func main() {
	app := newApp()

	app.Listen(":8080")
}

func newApp() *iris.Application {
	app := iris.New()
	mvc.Configure(app.Party("/user"), func(m *mvc.Application) {
		m.Handle(new(userController))
	})

	return app
}

type userController struct{}

func (c *userController) Get(req struct {
	Start int `url:"start"`
	Limit int `url:"limit"`
}) string {
	return fmt.Sprintf("Start: %d | Limit: %d\n", req.Start, req.Limit)
}

What version of Iris are u using? You should always try master branch before opening a new issue:

$ cd your_project
$ go get -u github.com/kataras/iris/v12@master
$ go run .

@ghost
Copy link
Author

ghost commented Jul 24, 2020

Sorry for that i missed some points.

I found that if controller has a public struct field, it happened.

type userController struct{
  Foo struct{}
}

@kataras

@kataras
Copy link
Owner

kataras commented Jul 24, 2020

@ghost-void This is an expected behavior and it's the correct one. Let me explain,

This happens because Foo is not bind-ed to any dependency, so Iris will try to bind it from the request, just like the req struct {Start, Limit} in your controller's method. The problem is that because Foo is empty, the underline ctx.ReadBody, which controller will use to bind the Foo, returns an error because by-default Controller -> Unknown Dependency -> Can be readen by body -> ctx.ReadBody -> GET: -> ctx.ReadForm in your case it does not allow unknown url fields to be passed, and Foo does not contain any url or form struct fields tags, so it stops the execution, so the Get(req...) is not fired at all. At serve-time, if a struct field is request-scoped (like that Iris one, the automatic payload binding) then it starts with it, so calling method can have access to the controller's fields as expected. Try to log the error by registering an ErrorHandler and see by yourself:

mvc.Configure(app.Party("/user"), func(m *mvc.Application) {
	m.HandleError(func(ctx iris.Context, err error) {
                // this will print: schema: invalid path "start" (and 1 other error)
		println(err.Error()) 
	})

	m.Handle(new(userController))
})

Solutions by you:

  1. Register a dependency for that Foo field with mvc.Application.Register or by Handle(&UserController{Foo: struct{}{}})
  2. or remove it if it's useless

What I can do:

  • You can register an error handler and allow the execution to continue but this is not allowed for struct fields (because if a field is empty (or nil!) and execution continues you may have a runtime panic if u try to access that field from the controller's method, and Iris is really careful protecting ur controller from things like that). So, I can just allow to have control over struct fields errors too so you can ignore the error and method executed with your expected results.
  • As explained above, by-default form decoder does NOT allow undefined url query arguments and will throw an error, which can be ignored on an iris.Handler, but in controller if that error came from a struct, then it stops the execution as noted before. I can add a setting which form bindIng can just continue and do its work, even if a url parameter is not defined in your struct.

Stay tuned, I will push examples and implement both of the above ^

@kataras kataras added this to the v12.2.0 milestone Jul 24, 2020
kataras added a commit to iris-contrib/schema that referenced this issue Jul 24, 2020
…users and ignore unknown url query parameters instead of giving an ErrPath

relative to: kataras/iris#1566
@kataras
Copy link
Owner

kataras commented Jul 24, 2020

@ghost-void if you upgrade to the latest /v12@master branch the above code you post will work just fine, but please consider by notes above. Thanks a lot for opening this issue!

kataras added a commit that referenced this issue Jul 24, 2020
@ghost
Copy link
Author

ghost commented Jul 25, 2020

It works, nice job!

kataras added a commit that referenced this issue Jul 26, 2020
Former-commit-id: fe2dbe46ac8153ef4db0b5788794bf907b98baad
@kataras
Copy link
Owner

kataras commented Jul 26, 2020

Thanks @ghost-void !

@kataras kataras closed this as completed Aug 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant