Server - store list of established connections.

[luboszk]

Hi! I'm trying to use your library in my project.

I'm working on Telnet Chat and I'm using ShellHandler for that.
The issue which I have is that Server doesn't provide access to new connections in any way.

I was thinking about copying your code for ListenAndServe, Serve and handle methods to my project as implementations of some supertype which will embed telnet.Server but it require a lot of code to be copied. What do you think will be the best choice to solve that?

How do you feel about changing handle method into exported?
Then I would have to implement only ListenAndServer and Serve methods in my own struct.

Another way is just to implement that functionality directly in go-telnet, are you OK with merging change like that?

A third way is to add current connection into a context which then is provided into Handler.
I have own handler which I'm using so that will also solve my issue. Also, it makes sense to add into context current conn, for example, if someone will do other things based on the source of the connection.
To be honest that 3rd option makes the most sense for me. Doesn't require a lot of changes and provides the biggest value added.

[reiver]

@diltram I tried to make my telnet package feel like Go's built-in "net/http" package.

Thus, my telnet.Server struct is meant to feel like the http.Server struct.

The telnet.Server struct (and the http.Server struct) don't represent the underlying TCP connection(s).

It is the (non-exported) telnet.Server.handle() method that has access to the TCP connection.

(Well, that and the telnet.Server.Serve() method, that calls telnet.Server.handle().)

[reiver]

@diltram Why do you want access to the underlying TCP connection?

What are you trying to do?

(If I understood what you are trying to accomplish, it might make it easier to figure out how to best design this.)

[reiver]

@diltram Actually... the Go built-in "net/http" package lets you access the underlying TCP connection with http.Hijacker

Could implement something similar to that for my telnet package.

It would make it so my telnet package still feels like "net/http".

[reiver]

@diltram So, what I'm thinking is something like this:

func myServeTELNET(ctx telnet.Context, w telnet.Writer, r telnet.Reader) {

    hj, ok := w.(telnet.Hijacker)
    if !ok {
        io.WriteString(w, "TELNET server doesn't support hijacking\r\n")
        return
    }

    conn, err := hj.Hijack()
    if nil != err {
        io.WriteString("ERROR: ")
        io.WriteString(w, err.Error())
        io.WriteString("\r\n")
        return
    }

    //@TODO: Do something with the net.Conn
}

But... me understanding why you want to get access to the underlying net.Conn (rather than just using telnet.Writer and telnet.Reader) could suggest that maybe telnet.Writer or telnet.Reader (for example) need more methods.

So... what did you need to use from net.Conn?...

• net.Conn.LocalAddr() Addr
• net.Conn.RemoteAddr() Addr
• net.Conn.SetDeadline(t time.Time) error
• net.Conn.SetReadDeadline(t time.Time) error
• net.Conn.SetWriteDeadline(t time.Time) error

[ghost]

I did an experiment modifying these two files:

modified:   vendor/github.com/reiver/go-telnet/context.go
modified:   vendor/github.com/reiver/go-telnet/server.go

I allowed for the setting of nett.Conn on telnet.Context, and set it inside server.go.
In response to your query, net.Conn.RemoteAddr() Addr is my primary concern in identifying which connection the command handler was triggered by. In the changes I proposed in #7 I'm needing the same information from net.Conn.RemoteAddr() Addr, but it makes more sense to include a reference to the connection rather than extract a single value.

If you are interested, I'll do a PR with what I've described, which should allow closing #6 and #7, after shipping what I'm currently working on.

[sebastien-rosset]

One use case would be to gracefully notify clients that connections are going to be closed.
Golang net.http.Server supports Shutdown() and Close() to terminate connections.