Looping issue in function

Viewed 168

I have a small TCP server running where a user can connect via telnet. There is a "menu" option loop (e.g. triggered single char input), and selecting the "L" option allows the connected user to "leave a message."

However:

  1. When making a selection (L) from the connect menu -- it duplicates/repeats the menu
  2. Then, an additional/unnecessary key-press is required to see the type echo'd back to the user

I suspect I've got looping problems, but I'm at a loss to where it's breaking down.

Any ideas?

package main

import (
    "bytes"
    "fmt"
    "log"
    "net"
)

const (
    port = "5555"
)

var arr []string

type Client struct {
    c        net.Conn
    dataType string
    menuCurr string
}

func NewClient() *Client {
    return &(Client{})

}

func waitForInput(didInput chan<- bool) {
    // Wait for a valid input here

    didInput <- true
}

func main() {

    log.Printf("Hello Server!")
    service := ":5555"
    tcpAddr, error := net.ResolveTCPAddr("tcp", service)
    if error != nil {
        log.Printf("Error: Could not resolve address")
    } else {
        netListen, error := net.Listen(tcpAddr.Network(), tcpAddr.String())
        if error != nil {
            log.Fatal(error)
        } else {
            defer netListen.Close()
            for {
                log.Printf("Waiting for clients")
                conn, error := netListen.Accept()
                if error != nil {
                    log.Print("Client error: ", error)
                } else {
                    log.Printf("Client connected %s -> %s \n", conn.RemoteAddr(), conn.LocalAddr())
                    go handler(conn)
                }
            }
        }
    }
}

func removeClient(conn net.Conn) {

    log.Printf("Client %s disconnected", conn.RemoteAddr())
    conn.Close()
}

func handler(conn net.Conn) {

    defer removeClient(conn)
    errorChan := make(chan error)
    dataChan := make(chan []byte)

    // Set defaults for incoming connections
    var s *Client
    s = NewClient()
    s.c = conn
    s.dataType = "key"
    s.menuCurr = "connect" // first menu every user sees

    go readWrapper(conn, dataChan, errorChan)
    r := bytes.NewBuffer(make([]byte, 0, 1024))

    // default menu
    fmt.Fprintf(conn, "Select an option:\r\n\n[L] Leave Message\r\n[!] Disconnect\r\n\nCmd? ")

    for {
        select {
        case data := <-dataChan:

            if s.menuCurr == "connect" {
                fmt.Fprintf(conn, "Select an option:\r\n\n[L] Leave Message\r\n[!] Disconnect\r\n\nCmd? ")
            }
            if s.menuCurr == "message" {
                fmt.Fprintf(conn, "Type a mesage. Escape to quit. \r\n\n")
            }
            // "key" responds to single character input
            if s.dataType == "key" {
                switch string(data) {
                default:
                    fmt.Println("client hit invalid key...")
                    continue
                case "L", "l":
                    s.dataType = "text"
                    s.menuCurr = "message"
                    continue
                case "!":
                    fmt.Fprintf(conn, " Bye!")
                    fmt.Println("client chose to exit...")
                    break
                }
            }

            // "Text" allows for free typing
            if s.dataType == "text" {
                for {
                    select {
                    case data := <-dataChan:
                        fmt.Fprintf(conn, string(data))
                        if bytes.Equal(data, []byte("\r\n")) || bytes.Equal(data, []byte("\r")) {
                            fmt.Fprintf(conn, "you typed: %q\r\n", r.String())
                            r.Reset()
                            s.dataType = "key"
                            s.menuCurr = "connect"
                            break
                        }
                        if bytes.Equal(data, []byte("\033")) {
                            fmt.Fprintf(conn, "\r\nAborted!\r\n")
                            r.Reset()
                            s.dataType = "key"
                            s.menuCurr = "connect"
                            break
                        }
                        r.Write(data)
                    }
                    log.Printf("Client %s sent: %q", conn.RemoteAddr(), string(data))

                    if s.menuCurr == "connect" {
                        break
                    }
                }
                continue
            }

        case err := <-errorChan:
            log.Println("An error occured:", err.Error())
            return
        }
        conn.Close()
    }

}

func readWrapper(conn net.Conn, dataChan chan []byte, errorChan chan error) {
    for {

        buf := make([]byte, 1024)
        reqLen, err := conn.Read(buf)
        if err != nil {
            errorChan <- err
            return
        }
        dataChan <- buf[:reqLen]

    }

}


2 Answers

So you almost got it!

  1. The reason for the need of a extra keypress is due to your select case not processing until it receives another character. Basically you were lagging one character behind. You also need to be aware of this when you are collecting the message from the user.

Here is is what i was able to get working. My telnet client doesn't send until I hit return (i am on windows why \r\n instead of just \n) so i needed to strip those extra characters.

package main

import (
    "bytes"
    "fmt"
    "log"
    "net"
    "strings"
)

const (
    port = "5555"
)

var arr []string

type Client struct {
    c        net.Conn
    dataType string
    menuCurr string
}

func NewClient() *Client {
    return &(Client{})

}

func waitForInput(didInput chan<- bool) {
    // Wait for a valid input here

    didInput <- true
}

func main() {

    log.Printf("Hello Server!")
    service := ":5555"
    tcpAddr, error := net.ResolveTCPAddr("tcp", service)
    if error != nil {
        log.Printf("Error: Could not resolve address")
    } else {
        netListen, error := net.Listen(tcpAddr.Network(), tcpAddr.String())
        if error != nil {
            log.Fatal(error)
        } else {
            defer netListen.Close()
            for {
                log.Printf("Waiting for clients")
                conn, error := netListen.Accept()
                if error != nil {
                    log.Print("Client error: ", error)
                } else {
                    log.Printf("Client connected %s -> %s \n", conn.RemoteAddr(), conn.LocalAddr())
                    go handler(conn)
                }
            }
        }
    }
}

func removeClient(conn net.Conn) {

    log.Printf("Client %s disconnected", conn.RemoteAddr())
    conn.Close()
}

func handler(conn net.Conn) {

    defer removeClient(conn)
    errorChan := make(chan error)
    dataChan := make(chan []byte)

    // Set defaults for incoming connections
    var s *Client
    s = NewClient()
    s.c = conn
    s.dataType = "key"
    s.menuCurr = "connect" // first menu every user sees

    go readWrapper(conn, dataChan, errorChan)
    r := bytes.NewBuffer(make([]byte, 0, 1024))

    // default menu
    fmt.Fprintf(conn, "Select an option:\r\n\n[L] Leave Message\r\n[!] Disconnect\r\n\nCmd? ")

    for {
        select {
        case data := <-dataChan:
            // notice how i removed the current menu state

            // "key" responds to single character input
            if s.dataType == "key" {
                t := strings.TrimSuffix(strings.TrimSuffix(string(data), "\r\n"), "\n")
                switch t {
                default:
                    fmt.Println("client hit invalid key...")
                    // remove the continue since the menu prints at the bottom
                    // continue
                case "L", "l":
                    s.dataType = "text"
                    s.menuCurr = "message"

                    // notice the message here and the break instead of the continue.
                    // if we use continue instead it will wait until your user sends something
                    // with a break instead it will fall through and start collecting the text
                    fmt.Fprintf(conn, "Type a mesage. Escape to quit. \r\n\n")
                    break
                case "!":
                    fmt.Fprintf(conn, " Bye!")
                    fmt.Println("client chose to exit...")
                    // tell current menu to exit
                    s.menuCurr = "exit"
                }
            }

            // "Text" allows for free typing
            if s.dataType == "text" {
                for {
                    select {
                    case data := <-dataChan:
                        fmt.Fprintf(conn, string(data))
                        if bytes.Equal(data, []byte("\r\n")) || bytes.Equal(data, []byte("\r")) {
                            fmt.Fprintf(conn, "you typed: %q\r\n", r.String())
                            r.Reset()
                            s.dataType = "key"
                            s.menuCurr = "connect"
                            break
                        }
                        if bytes.Equal(data, []byte("\033")) || bytes.Equal(data, []byte("\033\r\n")) {
                            fmt.Fprintf(conn, "\r\nAborted!\r\n")
                            r.Reset()
                            s.dataType = "key"
                            s.menuCurr = "connect"
                            break
                        }
                        r.Write(data)
                    }
                    log.Printf("Client %s sent: %q", conn.RemoteAddr(), r.String())
                    if s.menuCurr == "connect" {
                        break
                    }
                }
            }

            if s.menuCurr == "connect" {
                fmt.Fprintf(conn, "Select an option:\r\n\n[L] Leave Message\r\n[!] Disconnect\r\n\nCmd? ")
            }
            // fall through statement to close connection
            if s.menuCurr == "exit" {
                break
            }

            // otherwise continue printing menu for invalid submissions
            continue

        case err := <-errorChan:
            log.Println("An error occured:", err.Error())
            return
        }
        fmt.Println("closing")
        break
    }
    conn.Close()
}

func readWrapper(conn net.Conn, dataChan chan []byte, errorChan chan error) {
    for {

        buf := make([]byte, 1024)
        reqLen, err := conn.Read(buf)
        if err != nil {
            errorChan <- err
            return
        }
        dataChan <- buf[:reqLen]

    }

}

When using a stock telnet command-line program, it will typically negotiate with the server to run in LINEMODE. In this mode, to save on bandwidth, user input will be buffered. Data is flushed (i.e. sent to the server) when the user hits:

  • Enter; or
  • Ctrl-D

You can use your server in its current state, but you'd have to type the keystrokes: L followed by Ctrl-D (to force a flush). Then the client will receive the expected server response: type your message.

Your server-code is expecting a single key-stroke (L, !), so if you are hitting L followed by Enter - then the message received by the server will be L\r\n, which is where your server code is tripping up:

if s.dataType == "key" {
    switch string(data) {
        default:
            fmt.Printf("client hit invalid key... (%q)\n", string(data)) // <- `L\r\n`
            continue
        case "L", "l":
            s.dataType = "text"
            s.menuCurr = "message"
            continue
        case "!":
            fmt.Fprintf(conn, " Bye!")
            fmt.Println("client chose to exit...")
            break
    }
}
Related