Simple Golang TCP server

Question

I've written an itty bitty tcp server, for educational purposes.

The program outputs a file if you input the correct hash (or whatever you set the environment variable to).

I've performed tests on the application through netcat ( nc localhost 9000 ) and everything is fine, the application works as intended, both in my docker container and running "standalone".

Here's the 80 lines of code.

package main

import (
        "fmt"
        "io"
        "io/ioutil"
        "net"
        "bytes"
        "os"
        "log"
)

var OUTPUT_FILE = os.Getenv("OUTPUT_FILE")
var SECRET_HASH = os.Getenv("SECRET_HASH")

func handleError(e error) {
        if e != nil {
                log.Fatal(e)
                panic(e)
        }
}

func printFromFile(connection net.Conn) {
        /* Read file */
        data, err := ioutil.ReadFile(OUTPUT_FILE)
        handleError(err)

        /* Close connection */
        io.WriteString(connection, string(data))
        connection.Close()
}

func invalidHashMessage(connection net.Conn) {
        io.WriteString(connection, fmt.Sprint("Invalid hash\n"))
}

func handleMessage(connection net.Conn) {
        buffer := make([]byte, 1024)

        reqLen, err := connection.Read(buffer)
        if reqLen == 0 {
                connection.Close()
                return
        }

        handleError(err)

        n := bytes.Index(buffer, []byte{0})
        message := string(buffer[:n-1])

        if message != SECRET_HASH {
                invalidHashMessage(connection)
                handleMessage(connection)
        } else {
                printFromFile(connection)
        }
}

func main() {

        if OUTPUT_FILE == "" || SECRET_HASH == "" {
                fmt.Print("You need to supply output file and hash to serve application\n")
                os.Exit(1)
        }

        /* Start listener */
        ln, err := net.Listen("tcp", ":9000")
        handleError(err)
        defer ln.Close()


        for {
                conn, err := ln.Accept()
                handleError(err)

                io.WriteString(conn, "Please type unique identifier:")
                go handleMessage(conn)
        }
}

So.. my questions are.

• How would you handle concurrent connections? Making it real pretty. I have a certain feeling this could be improved upon, or is this way totally ok?
• Am i formatting the message correctly (the file content) so that available clients/terminals that access this little application all works the same?

• When i run this application through strace i get the following message which is saying that the port is already taken (?), but the application runs just fine. Trying to understand why strace outputs this.

   accept4(3, 0xc420051c00, [112], SOCK_CLOEXEC|SOCK_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable)
  

Answer 1

main

On very first lines of main you test for provided enviroment variables and write error message to stdout if any are missing. It is more common to write error messages to stderr. stdout is for main program output (none in your case).

You can write to stderr with io.Stderr:

io.WriteString(os.Stderr, "You need to supply output file and hash to serve application.\n")
os.Exit(1)

Note: os.Exit won't call any deferred functions

You can read more about stdout and stderr usage at Stack Overflow and in this article.

---

In a for loop you start a routine to handle each new connection separetly, so nothing will block. But one line before the go handleMessage(conn) you write to conn with io.WriteString. This is blocking I/O operation and in some cases it may block for a long time.

I suggest you to move all interactions with new conn to a separate routine.

handleMessage

handleMessage function is an entry point for routine that handles new connections. Instead of calling conn.Close() multiple times across different functions consider using a defer statement:

function handleMessage(conn net.Conn) {
    defer conn.Close()
    // function body goes here
}

This way when handleMessage function returns or panics the conn will be closed.

---

buffer := make([]byte, 1024)

You allocate 1024 bytes long buffer to read provided input from conn. If selected hash value is greater then 1024 you won't be able to read it and all connections will fail with non matching hashes.

buffer := make([]byte, len(SECRET_HASH))

---

You call conn.Read method to read provided input. The Read documentation states only specific details, to find out more let's see io.Reader interface documentation.

Some highlights:

Read reads up to len(p) bytes into p. It returns the number of bytes read (0 <= n <= len(p)) and any error encountered. Even if Read returns n < len(p), it may use all of p as scratch space during the call. If some data is available but not len(p) bytes, Read conventionally returns what is available instead of waiting for more.

As stated by documentation Read method can return too early. To handle this let's use io.ReadFull, which returns error only when fewer bytes were read.

_, err := io.ReadFull(conn, buffer)
if err != nil {
    // handle error
}

printFromFile

Reading whole file into memory has several pitfalls:

• files larger than availiable memory won't be processed
• on large number of concurent connections program will use lost of memory

To resolve this issues you may use io.Copy. It will read file in small chunks and write it directly to conn until EOF or error happens. nil error value indicates all gone fine.

func printFromFile(conn net.Conn) {
    file, err := io.Open(OUTPUT_FILE)
    if err != nil {
        // handle error
    }
    defer file.Close()

    _, err = io.Copy(conn, file)
    if err != nil {
      // handle error
    }
}

Example rewrite

package main

import(
    "io"
    "log"
    "net"
    "os"
)

var (
    outputFile = os.Getenv("OUTPUT_FILE")
    hash = os.Getenv("SECRET_HASH")
)

func handleConn(conn net.Conn) (err error) {

    defer conn.Close()

    _, err = io.WriteString(conn, "Please type unique identifier:")
    if err != nil { return }

    buf := make([]byte, len(hash))

    _, err = io.ReadFull(conn, buf)
    if err != nil { return }

    if string(buf) != hash {
        _, err = io.WriteString(conn, "Invalid hash\n")
        return
    }

    file, err := os.Open(outputFile)
    if err != nil { return }
    defer file.Close()

    _, err = io.Copy(conn, file)
    return
}

func main() {

    if len(outputFile) == 0 || len(hash) == 0 {
        io.WriteString(os.Stderr, "You need to supply output file and hash to serve application.\n")
        os.Exit(1)
    }

    ln, err := net.Listen("tcp", ":8080")
    if err != nil {
        log.Println(err)
        os.Exit(1)
    }
    defer ln.Close()

    for {
        conn, err := ln.Accept()
        if err != nil {
            log.Println(err)
            continue
        }

        go func(conn net.Conn) {
            err := handleConn(conn)
            if err != nil {
                log.Println(err)
            }
        }(conn)
    }
}

Points of interest:

• simplified error handling
• less functions (main, handleConn)
• less memory usage
• less number of imported packages