alexedwards / scs

HTTP Session Management for Go
MIT License
2.13k stars 166 forks source link

use of closed network connection #31

Closed dxvgef closed 5 years ago

dxvgef commented 6 years ago

I use SCS to implement session functionality in the echo framework, but I delete the sessionid in cookie and then refresh the browser and output the error:

redigo: unexpected type for Bytes, got type []interface {}
redigo: unexpected response line (possible server error or unsupported concurrent read by application)
write tcp 127.0.0.1:49741->127.0.0.1:6379: use of closed network connection

My code:

//Connect Redis
func connectRedis() (redis.Conn, error) {
    conn, err := redis.Dial("tcp", "127.0.0.1:6379")
    if err != nil {
        return nil, err
    }
    return conn, err
}

//Configure SCS
func SetSession() error {
    redisConn, err := connectRedis()
    if err != nil {
        return err
    }
    redisPool := redisstore.New(redis.NewPool(func() (redis.Conn, error) {
        return redisConn, nil
    }, 1))

    SessionManager = scs.NewManager(redisPool)
    SessionManager.Name("sessionid")
    SessionManager.Path("/")
    SessionManager.Lifetime(60 * time.Minute)
    SessionManager.Secure(false)
    SessionManager.HttpOnly(true)
    SessionManager.IdleTimeout(20 * time.Minute)
    SessionManager.Persist(true)
    return nil
}

//Echo middleware
func SessionMiddleware() echo.MiddlewareFunc {
    return func(next echo.HandlerFunc) echo.HandlerFunc {
        return func(ctx echo.Context) error {
            //update session idletime
            session := SessionManager.Load(ctx.Request())
            err := session.Touch(ctx.Response().Writer)
            if err != nil {
                return err
            }
            return next(ctx)
        }
    }
}
jpfluger commented 6 years ago

I could not replicate your error. I tried a version of your code (see below) that I tested by

  1. deleting the session id in redis, and
  2. in the browser (via clearing all cookies - using Chrome).
  3. then stress-tested with vagera for good-measure

Maybe...

Here's the middleware I used for testing:

package midware

import (
    "github.com/labstack/echo"
    "github.com/labstack/echo/middleware"
    "github.com/alexedwards/scs"
    "fmt"
    "github.com/alexedwards/scs/stores/redisstore"
    "github.com/garyburd/redigo/redis"
    "time"
        "net/http/httputil"
)

type SessionInitializeConfig struct {
    // Skipper defines a function to skip middleware.
    Skipper middleware.Skipper
    // the scs session manager
    SessionManager *scs.Manager
}

var (
    DefaultSessionInitializeConfig = SessionInitializeConfig{
        Skipper: middleware.DefaultSkipper,
        // hardcoded encryption key for testing - not production
        // https://github.com/alexedwards/scs
        // to create random key: https://stackoverflow.com/questions/22892120/how-to-generate-a-random-string-of-a-fixed-length-in-golang
        SessionManager: scs.NewCookieManager("u46IpCV9y5Vlur8YvODJEhgOY8m9JVE4"),
    }
)

func SessionInitialize() echo.MiddlewareFunc {
    return SessionInitializeWithConfig(DefaultSessionInitializeConfig)
}

func NewSessionManagerRedis() *scs.Manager {
    engine := redisstore.New(
        &redis.Pool{
            MaxIdle: 10,
            Dial: func() (redis.Conn, error) {
                return redis.Dial("tcp", "127.0.0.1:6379")
            },
        },
    )

    sessionManager := scs.NewManager(engine)
    sessionManager.Name("sessionid")
    sessionManager.Path("/")
    sessionManager.Lifetime(60 * time.Minute)
    sessionManager.Secure(false)
    sessionManager.HttpOnly(true)
    sessionManager.IdleTimeout(20 * time.Minute)
    sessionManager.Persist(true)

    return sessionManager
}

func SessionInitializeWithConfig(config SessionInitializeConfig) echo.MiddlewareFunc {
    // Defaults
    if config.Skipper == nil {
        config.Skipper = DefaultSessionInitializeConfig.Skipper
    }
    if config.SessionManager == nil {
        config.SessionManager = DefaultSessionInitializeConfig.SessionManager
    }

    return func(next echo.HandlerFunc) echo.HandlerFunc {
        return func(c echo.Context) error {
            if config.Skipper(c) {
                return next(c)
            }

            session := config.SessionManager.Load(c.Request())
            session.Touch(c.Response())

            // NOTE: this is enough to initialize the cookie
            // return next(c)

            // ----------------
            // Begin tests

            counter, err := session.GetInt64("counter")
            if err != nil {
                counter = 0
            }
            counter++

            session.PutInt64(c.Response(), "counter", counter)
            session.PutString(c.Response(), "count-as-string", fmt.Sprintf("count is %d", counter))

            fmt.Printf("%+v\n", session)

            requestDump, err := httputil.DumpRequest(c.Request(), true)
            if err != nil {
                fmt.Println(err)
            }
            fmt.Println(string(requestDump))

            // End tests
            // ----------------

            return next(c)
        }
    }
}

Create the redis store and invoke with

sessionManager := midware.NewSessionManagerRedis()
e.Use(midware.SessionInitializeWithConfig(midware.SessionInitializeConfig{SessionManager: sessionManager}))
jpfluger commented 6 years ago

As I'm getting familiar with this software, I hadn't seen Use. Good reference as well, which can be wrapped in an echo function as discussed in #15. The one I volunteered follows the default echo middleware paradigm.

jpfluger commented 6 years ago

Also because the Opts is not exposed with a public Idletimeout, we can't write the equivalent of Use for echo using the default echo Logger.

alexedwards commented 6 years ago

I haven't been able to replicate this either. Is it still causing a problem?

alexedwards commented 5 years ago

Closing due to age and lack of further information. Please reopen if it continues to be a problem with the latest version.