Binary package CRLF problem [RE-OPENED]

Hey @leogazio,

I noticed you are using:

reader := bufio.NewReader(c.conn)

and then wrapping that in another buffered reader.

I’m not sure if it will fix it, but try this first:

reader := bytes.NewReader(c.conn)

Edit:

Also please check my updated code, since I replaced some of the parts like this:

if hdr[0] != 0x78 && hdr[1] != 0x78 { ... }

With:

if hdr[0] != 0x78 || hdr[1] != 0x78 { ... }
1 Like

Where you want to read exaxtly n bytes, use io.ReadFull witn a slices of n bytes.

If your program hangs, send SIGQUIT (or hit Ctrl-\) to have it quit and print out the stacktrace.
From there you can inspect where have your program stuck.

2 Likes

I tried to replace this piece;

reader := bytes.NewReader(c.conn)

But I’m getting a compilation error;
".\servidor_tcp.go:56: cannot use c.conn (type net.Conn) as type []byte in argument to bytes.NewReader"

Seems like bytes.NewReader used []byte type argument. Is there a way to fix it?

Now my code is like that;

	reader := bytes.NewReader(c.conn)

	for {
		var hdr [3]byte
		r := bufio.NewReader(reader)

Tnx.

Oh whoops, didn’t even think about that, just leave it as 1 buffered reader and for the first part, I suggest using io.ReadFull from the buffered reader like others have suggested :slight_smile:

Edit:

So basically the first part would be something like this:

var hdr [3]byte

r := bufio.NewReader(c.conn)

// Read into the header.
n, err := io.ReadFull(r, hdr[:])

// Make sure 3 bytes were read into the header.
if n < 3 || err != nil && err != io.EOF {
	log.Fatalln("invalid packet header")
}
1 Like

I’m still getting the same problem, below full code;

// Read client data from channel
func (c *Client) listen() {
	for {
		var hdr [3]byte

		r := bufio.NewReader(c.conn)

		// Read into the header.
		n, err := io.ReadFull(r, hdr[:])

		// Make sure 3 bytes were read into the header.
		if n < 3 || err != nil && err != io.EOF {
			log.Fatalln("invalid packet header")
		}

		// Make sure first 2 bytes are byte 78.
		if hdr[0] != 0x78 || hdr[1] != 0x78 {
			log.Fatalln("packet header start is not 78 78")
		}

		// Get packet length.
		ln := int(hdr[2])

		data := make([]byte, ln)
		n, err = r.Read(data)
		if n != ln || err != nil {
			log.Fatalln("error reading packet data, %v", err)
		}

		// Extract the crc from the last 2 bytes of the data.
		//	crc := data[len(data)-2:]

		// Read the final 2 bytes from the packet's end.
		end := make([]byte, 2)
		n, err = r.Read(end)
		if n != n || err != nil && err != io.EOF {
			log.Fatalln("error reading packet data, %v", err)
		}

		// Make sure the 2 end bytes are correct.
		if end[0] != 0x0d && end[1] != 0x0a {
			log.Fatalln("packet end is not 0x0d 0x0a")
		}

		strpacote := fmt.Sprintf("%x%x%x", hdr, data, end)
		c.Server.onNewMessage(c, strings.ToUpper(strpacote))
	}
}

Tnx.

Can you try to explain a little better what the actual problem is?

Just because this is confusing to me:

Because if it stopped execution, how are you getting to the end and receiving the right packet?

Can you please also display any actual errors you are receiving?

I shot this print;

Ah ok,

Can you run it again and print the error message again after replacing this:

log.Fatalln("invalid packet header")

With this:

log.Fatalf("invalid packet header %v\n", err)
1 Like

2017/02/16 04:20:16 invalid packet header EOF

Oh I’m pretty sure that’s just happening because you are inside of a for loop and after you read the packet, the next time you try to read again, there’s nothing else to read.

For example:

Server:

package main

import (
	"bufio"
	"fmt"
	"io"
	"log"
	"net"
)

func handleConn(c net.Conn) {
	defer c.Close()

	r := bufio.NewReader(c)

	for {
		var hdr [3]byte

		// Read into the header.
		n, err := io.ReadFull(r, hdr[:])

		// Make sure 3 bytes were read into the header.
		if n < 3 || err != nil && err != io.EOF {
			log.Fatalf("invalid packet header %v\n", err)
		}

		// Make sure first 2 bytes are 0x78.
		if hdr[0] != 0x78 || hdr[1] != 0x78 {
			log.Fatalln("packet header start is not 0x78 0x78")
		}

		// Get packet length.
		ln := int(hdr[2])

	data := make([]byte, ln)

	n, err = io.ReadFull(r, data)
	if n < 2 || err != nil && err != io.EOF {
		log.Fatalf("error reading packet data, %v\n", err)
	}

		// Extract the crc from the last 2 bytes of the data.
		crc := data[len(data)-2:]

		// Read the final 2 bytes from the packet's end.
		end := make([]byte, 2)

		// Read the last 2 bytes.
		n, err = io.ReadFull(r, end)

		// Make sure 2 bytes were read into end.
		if n < 2 || err != nil && err != io.EOF {
			log.Fatalf("invalid packet end %v\n", err)
		}

		// Make sure the 2 end bytes are correct.
		if end[0] != 0x0d || end[1] != 0x0a {
			log.Fatalln("packet end is not 0x0d 0x0a")
		}

		fmt.Printf("header: %x\ndata: %x\ncrc: %x\nend: %x\n", hdr, data, crc, end)
	}
}

func main() {
	l, err := net.Listen("tcp", ":9000")
	if err != nil {
		log.Fatalln(err)
	}
	for {
		c, err := l.Accept()
		if err != nil {
			fmt.Println(err)
			continue
		}
		go handleConn(c)
	}
}

Client:

package main

import (
	"log"
	"net"
)

func main() {
	packets := []byte{
		0x78, 0x78, 0x0a, 0x13, 0x46, 0x00, 0x43, 0x00, 0x01, 0x00, 0x14, 0xcb, 0x23, 0x0d, 0x0a,
		0x78, 0x78, 0x0a, 0x14, 0x47, 0x00, 0x43, 0x00, 0x01, 0x00, 0x14, 0xcb, 0x23, 0x0d, 0x0a,
	}

	c, err := net.Dial("tcp", ":9000")
	if err != nil {
		log.Fatalln(err)
	}
	defer c.Close()

	_, err = c.Write(packets)
	if err != nil {
		log.Fatalln(err)
	}
}

You will get the output for the first 2 packets, but then an EOF.

1 Like

By the way with my example, if you want to constantly have different clients, just change the handleConn part to this so it just closes the client connection when an EOF occurs:

func handleConn(c net.Conn) {
	defer c.Close()

	r := bufio.NewReader(c)

	for {
		var hdr [3]byte

		// Read into the header.
		n, err := io.ReadFull(r, hdr[:])

		// Make sure 3 bytes were read into the header.
		if n < 3 || err != nil {
			if err == io.EOF {
				return
			}
			log.Printf("invalid packet header %v\n", err)
			return
		}

		// Make sure first 2 bytes are 0x78.
		if hdr[0] != 0x78 || hdr[1] != 0x78 {
			log.Println("packet header start is not 0x78 0x78")
			return
		}

		// Get packet length.
		ln := int(hdr[2])

		data := make([]byte, ln)

		n, err = io.ReadFull(r, data)
		if n < 2 || err != nil {
			log.Printf("error reading packet data, %v\n", err)
			return
		}

		// Extract the crc from the last 2 bytes of the data.
		crc := data[len(data)-2:]

		// Read the final 2 bytes from the packet's end.
		end := make([]byte, 2)

		// Read the last 2 bytes.
		n, err = io.ReadFull(r, end)

		// Make sure 2 bytes were read into end.
		if n < 2 || err != nil {
			log.Printf("invalid packet end %v\n", err)
			return
		}

		// Make sure the 2 end bytes are correct.
		if end[0] != 0x0d || end[1] != 0x0a {
			log.Println("packet end is not 0x0d 0x0a")
			return
		}

		fmt.Printf("header: %x\ndata: %x\ncrc: %x\nend: %x\n", hdr, data, crc, end)
	}
}
1 Like

But I can’t disconnect clients my friend, not in my situation, it’s a GPRS tracker which has 2 minute as transmission interval and send the first package only 2 minutes after the first message ack(login message), everytime I disconnect the client, it will try another connection, send another login message and only after 2 minutes will start working, if the network signal is bad, that will bring me lots of problems. I really need to do it without disconnecting any client. Everything is working, but there’s this EOF at the 0-byte like I’ve seen on here searching on Google.

I did this on this condition, to show what value has hdr [0] and [1];

		// Make sure first 2 bytes are byte 78.
		if hdr[0] != 0x78 || hdr[1] != 0x78 {
			log.Fatalln("packet header start is not 78 78\r\n" + fmt.Sprintf("Primeiro byte: %x - Segundo byte: %x", hdr[0], hdr[1]))
		}

This is what I got;

Tnx.

Hey guys, searching on Google I found this post;

What about this;

strings.TrimRight(input, "\r\n")

Seems like it takes only the last “\r\n”, and ignores if it’s found in the middle of the string, maybe treating the data as string only to use this TrimRight, and after that converting back to []byte…

Hey again @leogazio,

I think you misunderstand why you are receiving an EOF.
It’s occurring because the client itself is closing the connection after sending it’s packets.

In my example above, if you were to replace the client with this:

package main

import (
	"log"
	"net"
	"time"
)

func main() {
	c, err := net.Dial("tcp", ":9000")
	if err != nil {
		log.Fatalln(err)
	}
	defer c.Close()

	packets := []byte{
		0x78, 0x78, 0x0a, 0x13, 0x46, 0x00, 0x43, 0x00, 0x01, 0x00, 0x14, 0xcb, 0x23, 0x0d, 0x0a,
		0x78, 0x78, 0x0a, 0x14, 0x47, 0x00, 0x43, 0x00, 0x01, 0x00, 0x14, 0xcb, 0x23, 0x0d, 0x0a,
	}

	for {
		_, err = c.Write(packets)
		if err != nil {
			log.Fatalln(err)
		}

		time.Sleep(time.Second * 5)
	}
}

It would repeatedly send packets once every 5 seconds and only close the connection if you exit the program, so realistically, if you just simply keep the client open, the server won’t close the client’s connection (assuming you don’t have deadlines set on the server’s side).

However, if there’s a legitimate error, you would most likely want to close the client’s connection and usually an EOF or any other error would mean either the client is done or there’s an actual problem.

This EOF was not the only problem, I was getting millions of 0-byte if I comment the log calls. My problem was that for loop, I pasted that tcp server from an example, then after I cutted the for loop,I started just receiving only every client first package, my whole code below, I’m o here 4 days trying to make it works, and that was the only way I got it working;

package main

import (
	"bufio"
	"encoding/hex"
	"fmt"
	"io"
	"log"
	"net"
	"strings"
)

// Client holds info about connection
type TCPClient struct {
	conn   net.Conn
	Server *server
}

// TCP server
type server struct {
	clients                  []*TCPClient
	address                  string // Address to open connection: localhost:9999
	onNewClientCallback      func(c *TCPClient)
	onClientConnectionClosed func(c *TCPClient, err error)
	onNewMessage             func(c *TCPClient, message string)
}

// Read client data from channel
func (c *TCPClient) listen() {
	var hdr [3]byte

	r := bufio.NewReader(c.conn)

	// Read into the header.
	n, err := io.ReadFull(r, hdr[:])

	// Make sure 3 bytes were read into the header.
	if n < 3 || err != nil && err != io.EOF {
		GeraLog("Pacote quebrado: Invalid packet header.")
		go c.listen()
		return
	}

	// Make sure first 2 bytes are byte 78.
	if hdr[0] != 0x78 || hdr[1] != 0x78 {
		GeraLog("Pacote quebrado: Packet header start is not 78 78.")
		go c.listen()
		return
	}

	// Get packet length.
	ln := int(hdr[2])

	data := make([]byte, ln)
	n, err = r.Read(data)
	if n != ln || err != nil {
		GeraLog("Pacote quebrado: Error reading packet data.")
		go c.listen()
		return
	}

	// Read the final 2 bytes from the packet's end.
	end := make([]byte, 2)
	n, err = r.Read(end)
	if n != n || err != nil && err != io.EOF {
		GeraLog("Pacote quebrado: Error reading packet data.")
		go c.listen()
		return
	}

	// Make sure the 2 end bytes are correct.
	if end[0] != 0x0d && end[1] != 0x0a {
		GeraLog("Pacote quebrado: Packet end is not 0D 0A.")
		go c.listen()
		return
	}

	strpacote := fmt.Sprintf("%x%x%x", hdr, data, end)
	c.Server.onNewMessage(c, strings.ToUpper(strpacote))
	go c.listen()
}

// Send text message to client
func (c *TCPClient) Send(message string) error {
	hexbytes, _ := hex.DecodeString(message)
	_, err := c.conn.Write([]byte(hexbytes))
	return err
}

func (c *TCPClient) Conn() net.Conn {
	return c.conn
}

func (c *TCPClient) Close() error {
	return c.conn.Close()
}

// Called right after server starts listening new client
func (s *server) OnNewClient(callback func(c *TCPClient)) {
	s.onNewClientCallback = callback
}

// Called right after connection closed
func (s *server) OnClientConnectionClosed(callback func(c *TCPClient, err error)) {
	s.onClientConnectionClosed = callback
}

// Called when Client receives new message
func (s *server) OnNewMessage(callback func(c *TCPClient, message string)) {
	s.onNewMessage = callback
}

// Start network server
func (s *server) Listen() {
	listener, err := net.Listen("tcp", s.address)
	if err != nil {
		log.Fatal("Error starting TCP server.")
	}
	defer listener.Close()

	for {
		conn, _ := listener.Accept()
		client := &TCPClient{
			conn:   conn,
			Server: s,
		}
		go client.listen()
		s.onNewClientCallback(client)
	}
}

// Creates new tcp server instance
func NewTCP(address string) *server {
	server := &server{
		address: address,
	}

	server.OnNewClient(func(c *TCPClient) {})
	server.OnNewMessage(func(c *TCPClient, message string) {})
	server.OnClientConnectionClosed(func(c *TCPClient, err error) {})

	return server
}

Please give your opinions;

[EDIT]
If anyone figures out a better and more simple way to do it, I’ll appreciate it.

Tnx.

On those message mismatches, why fork a goroutine, why not just call c.listen() (or goto the top, or put all of it in a for cycle, and continue to the next round)?

1 Like

For loop for some reason is not working, it’s eventually reading 0-byte and giving eof even at the begining of the cycle…

Hey there, I’m still having that problem, and It happens like Benjamin said above, only when the client disconnects… Everytime client disconnects, listen event starts reading 0-byte giving EOF error. How to fix It?

When client disconnects, you’ll get io.EOF, and you shold terminate the loop - return from it.