Iris: [BUG] `authenticated-controller` `StopWithStatus` prevents overlapped `Controller` from executing

[AlbinoGeek]

Summary

When implementing the authenticated-controller example, one can see that the AuthDependency fires ctx.StopWithStatus(iris.StatusUnauthorized) when the AuthorizedUserController's dependency condition could not be met. Then, AuthorizedUserController's HandleHTTPError is fired, this prevents the UnauthorizedUserController's routes from being tested or fired.

While adding HandleHTTPError probably created this bug, I have to wonder if anything with UseError, OnError or HandleError would have had this issue before?

Versions, etc.

|Thing|Version|
|-|-|
|Golang|1.14.6 linux/amd64|
|Iris|master 9335345|
|Kernel|5.7.16-200.fc32.x86_64|
|OS|Fedora 32 Workstation|

Console Output

[WARN] 2020/08/27 16:33 401 248.645µs 192.168.0.1 GET /user
GET: /user user.AuthorizedUserController (./main.go:128)
     • iris/core/router.overlapRoute (/home/damon/go/pkg/mod/github.com/kataras/iris/[email protected]/core/router/api_builder.go:137)
     • iris.recover (/home/damon/go/pkg/mod/github.com/kataras/iris/[email protected]/middleware/recover/recover.go:30)
     • iris.logger (/home/damon/go/pkg/mod/github.com/kataras/iris/[email protected]/middleware/logger/logger.go:47)
     • user.AuthorizedUserController.Get (./controllers/user/Authorized.go:19)
[WARN] 2020/08/27 16:33 HandleHTTPError executed:
        mvc.Code = 401
        ctx.StatusCode = 401
[WARN] 2020/08/27 16:33 401 3.189619ms 192.168.0.1 GET /user
401: /user user.AuthorizedUserController (./main.go:128)
     • iris/core/router.overlapRoute (/home/damon/go/pkg/mod/github.com/kataras/iris/[email protected]/core/router/api_builder.go:137) ✓
     • iris.recover (/home/damon/go/pkg/mod/github.com/kataras/iris/[email protected]/middleware/recover/recover.go:30) ✓
     • iris.logger (/home/damon/go/pkg/mod/github.com/kataras/iris/[email protected]/middleware/logger/logger.go:47) ✓
     • user.AuthorizedUserController.HandleHTTPError (./controllers/user/Authorized.go:15) ✓ ✓

HTTP Transaction

GET [redacted]/user
DNT: 1
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64) -snip-
Accept: text/html,-snip-

HTTP/1.1 401
status: 401
content-encoding: gzip
content-type: text/html; charset=utf-8
vary: Accept-Encoding
x-request-id: 55d580ac-7e81-430b-b8e4-e997da2b4224
content-length: 1647
date: Thu, 27 Aug 2020 23:41:14 GMT
X-DNS-Prefetch-Control: off

Code

func AuthDependency(ctx iris.Context) Authenticated {
    session := sessionManager.Start(ctx)

    userID := session.GetUint64Default("user_id", 0)
    if userID == 0 {
        // If execution was stopped
        // any controller's method will not be executed at all.
        ctx.StopWithStatus(iris.StatusUnauthorized)
        return 0
    }

    return Authenticated(userID)
}

type BaseController struct{}

func (c *BaseController) BeforeActivation(b mvc.BeforeActivation) {
    // ...
}

func (c *BaseController) HandleHTTPError(ctx iris.Context, statusCode mvc.Code) mvc.Result {
    view := mvc.View{Code: int(statusCode)}
    golog.Warnf("HandleHTTPError executed:\n\tmvc.Code = %d\n\tctx.StatusCode = %d", view.Code, ctx.GetStatusCode())

    return nil // for testing sake
}

type AuthorizedUserController struct {
    base.BaseController
    CurrentUserID Authenticated
}

func (c *AuthorizedUserController) Get() mvc.Result {
    return mvc.View{
        Code: iris.StatusOK,
        Name: "user/index.html",
    }
}

type UnauthorizedUserController struct {
    base.BaseController
}

func (c *UnauthorizedUserController) Get(ctx iris.Context) mvc.Result {
    ctx.Redirect(fmt.Sprintf("%s/login", ctx.GetCurrentRoute().Path()), iris.StatusTemporaryRedirect)
    return nil
}

func main() {
// ...
    userRouter := app.Party("/user")
    userRouter.SetRegisterRule(iris.RouteOverlap)
    userApp := mvc.New(userRouter)
    userApp.Register(user.AuthDependency)
    userApp.Handle(new(user.AuthorizedUserController))
    userApp.Handle(new(user.UnauthorizedUserController))
// ...
}

[kataras]

@AlbinoGeek that's amazing, I was working for it before seen that issue, I've just pushed a commit. It wasn't a bug, it was intentionally (because of the versioning feature, read my latest commit to learn more), new test (which completes the above post of yours I think).

The example was also updated with comments: https://github.com/kataras/iris/blob/a6ec94e1a695828a8bf12c536e82d40112c68a4a/_examples/mvc/authenticated-controller/main.go#L63-L78

Indeed, the controller's method now receives the error passed by the dependency and can decide to modify it through context or output value. The dependency can also just use ctx.StopExecution to keep the 200 status ok and let the controller method to decide. (The same rules apply to any route, not only controllers*)

[AlbinoGeek]

Thank you again @kataras for personally seeing to this issue, where we now know a reproduction!

[kataras]

I thank YOU @AlbinoGeek!! Your help was gold for targeting the issue.

For everyone who is reading: it was the app.UseRouter/Use(iris.Compression) which led to that bug of stopping the execution on the internal core/router/#defaultOverlapFilter -> core/router/#overlapRoute because CompressResponseWriter.Reset() bool method was missing. That was fixed with the latest commit.

[AlbinoGeek]

Yup, it's fixed now! Just tested.