reiver / go-telnet

Package telnet provides TELNET and TELNETS client and server implementations, for the Go programming language, in a style similar to the "net/http" library that is part of the Go standard library, including support for "middleware"; TELNETS is secure TELNET, with the TELNET protocol over a secured TLS (or SSL) connection.
https://godoc.org/github.com/reiver/go-telnet
MIT License
266 stars 83 forks source link

Server - store list of established connections. #6

Open luboszk opened 6 years ago

luboszk commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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?...

majgis commented 6 years ago

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 commented 5 years ago

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.