From c7b389efa6887b85b86f2e2427d51b59a50098ae Mon Sep 17 00:00:00 2001 From: Unbewohnte Date: Mon, 7 Jun 2021 14:28:18 +0300 Subject: [PATCH] Somewhat reasonable error-handling --- README.md | 2 +- client/client.go | 9 +-- protocol/protocol.go | 131 +++++++++++++++++++++++-------------------- server/server.go | 22 ++++---- 4 files changed, 88 insertions(+), 76 deletions(-) diff --git a/README.md b/README.md index e947c23..224e888 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ Thus, with a connection and a way of communication, the server will send a filei - **VERY** slow; FIXED - [ ] - **VERY** expensive on resources; FIXED - [ ] - If `MAXFILEDATASIZE` is bigger than appr. 1024 - the packets on the other end will not be unmarshalled due to error ??; FIXED - [ ] -- Lack of proper error-handling; FIXED - [ ] +- Lack of proper error-handling; somewhat FIXED - [x] - Lack of information about the process of transferring (ETA, lost packets, etc.); FIXED - [ ] - No way to verify if the transferred file is not corrupted; FIXED - [ ] - No encryption; FIXED - [ ] diff --git a/client/client.go b/client/client.go index f0121cd..18054ef 100644 --- a/client/client.go +++ b/client/client.go @@ -139,10 +139,11 @@ func (c *Client) WritePieceOfFile(filePacket protocol.Packet) error { // Listens in an endless loop; reads incoming packages and puts them into channel func (c *Client) ReceivePackets() { for { - incomingPacket := protocol.ReadFromConn(c.Connection) - isvalid, _ := protocol.IsValidPacket(incomingPacket) - if !isvalid { - continue + incomingPacket, err := protocol.ReadFromConn(c.Connection) + if err != nil { + // in current implementation there is no way to receive a working file even if only one packet is missing + fmt.Printf("Error reading a packet: %s\nExiting...", err) + os.Exit(-1) } c.IncomingPackets <- incomingPacket } diff --git a/protocol/protocol.go b/protocol/protocol.go index 3900ece..0d59c62 100644 --- a/protocol/protocol.go +++ b/protocol/protocol.go @@ -34,39 +34,40 @@ type Packet struct { } // converts valid packet bytes into `Packet` struct -func ReadPacketBytes(packetBytes []byte) Packet { - // makes sure that the packet is ALWAYS less or equal to the maximum packet size - // this allows not to use any client or server checks - - //fmt.Println("READING packet: ", string(packetBytes)) - +func ReadPacketBytes(packetBytes []byte) (Packet, error) { var packet Packet err := json.Unmarshal(packetBytes, &packet) if err != nil { - fmt.Printf("Could not unmarshal the packet: %s\n", err) - return Packet{} + return Packet{}, fmt.Errorf("could not unmarshal packet bytes: %s", err) } - return packet + return packet, nil } // Converts `Packet` struct into []byte -func EncodePacket(packet Packet) []byte { +func EncodePacket(packet Packet) ([]byte, error) { packetBytes, err := json.Marshal(packet) if err != nil { - return []byte("") + return nil, fmt.Errorf("could not marshal packet bytes: %s", err) } - return packetBytes + return packetBytes, nil } // Measures the packet length -func MeasurePacket(packet Packet) uint64 { - packetBytes := EncodePacket(packet) - return uint64(len(packetBytes)) +func MeasurePacket(packet Packet) (uint64, error) { + packetBytes, err := EncodePacket(packet) + if err != nil { + return 0, fmt.Errorf("could not measure the packet: %s", err) + } + return uint64(len(packetBytes)), nil } // Checks if given packet is valid, returns a boolean and an explanation message func IsValidPacket(packet Packet) (bool, string) { - if MeasurePacket(packet) > uint64(MAXPACKETSIZE) { + packetSize, err := MeasurePacket(packet) + if err != nil { + return false, "Measurement error" + } + if packetSize > uint64(MAXPACKETSIZE) { return false, "Exceeded MAXPACKETSIZE" } if len(packet.FileData) > MAXFILEDATASIZE { @@ -74,7 +75,7 @@ func IsValidPacket(packet Packet) (bool, string) { } if strings.TrimSpace(string(packet.Header)) == "" { - return false, "Blank header" + return false, "Empty header" } return true, "" } @@ -87,69 +88,77 @@ func SendPacket(connection net.Conn, packet Packet) error { return fmt.Errorf("this packet is invalid !: %v; The error: %v", packet, msg) } - packetSize := MeasurePacket(packet) + packetSize, err := MeasurePacket(packet) + if err != nil { + return err + } // write packetsize between delimeters (ie: |727|{"HEADER":"PING"...}) connection.Write([]byte(fmt.Sprintf("%s%d%s", PACKETSIZEDELIMETER, packetSize, PACKETSIZEDELIMETER))) - // write an actual packet - connection.Write(EncodePacket(packet)) + // write the packet itself + packetBytes, err := EncodePacket(packet) + if err != nil { + return fmt.Errorf("could not send a packet: %s", err) + } + connection.Write(packetBytes) - //fmt.Println("Sending packet: ", string(EncodePacket(packet)), " Length: ", packetSize) return nil } -// Reads a packet from a connection by retrieving the packet length. Only once -// ASSUMING THAT THE PACKETS ARE SENT BY `SendPacket` method !!!! -func ReadFromConn(connection net.Conn) Packet { - var gotPacketSize bool = false +// Reads a packet from given connection. +// ASSUMING THAT THE PACKETS ARE SENT BY `SendPacket` function !!!! +func ReadFromConn(connection net.Conn) (Packet, error) { + var err error var delimeterCounter int = 0 - - var packetSizeStr string = "" + var packetSizeStrBuffer string = "" var packetSize int = 0 + for { - // still need to get a packetsize - if !gotPacketSize { - // reading byte-by-byte - buffer := make([]byte, 1) - connection.Read(buffer) - - // found a delimeter - if string(buffer) == PACKETSIZEDELIMETER { - delimeterCounter++ - - // the first delimeter is found, skipping the rest of the code - if delimeterCounter == 1 { - continue - } - } + // reading byte-by-byte + buffer := make([]byte, 1) + connection.Read(buffer) // no fixed time limit, so no need to check for an error + + // found a delimeter + if string(buffer) == PACKETSIZEDELIMETER { + delimeterCounter++ - // found the first delimeter, skip was performed, now reading an actual packetsize + // the first delimeter has been found, skipping the rest of the loop if delimeterCounter == 1 { - packetSizeStr += string(buffer) - } else if delimeterCounter == 2 { - // found the last delimeter, thus already read the whole packetsize - packetSize, _ = strconv.Atoi(packetSizeStr) - gotPacketSize = true + continue } - // skipping the rest of the code because we don`t know the packet size yet - continue } - // have a packetsize, now reading the whole packet - //fmt.Println("Got a packetsize!: ", packetSize) - packetBuffer := make([]byte, packetSize) - connection.Read(packetBuffer) + // found the first delimeter, skip was performed, now reading an actual packetsize + if delimeterCounter == 1 { + // adding a character of the packet size to the `string buffer`; ie: | <- read, reading now -> 1 23|PACKET_HERE + packetSizeStrBuffer += string(buffer) + + } else if delimeterCounter == 2 { + // found the last delimeter, thus already read the whole packetsize + // converting from string to int + packetSize, err = strconv.Atoi(packetSizeStrBuffer) + if err != nil { + return Packet{}, fmt.Errorf("could not convert packet size into integer: %s", err) + } + // packet size has been found, breaking from the loop + break + } + } - packet := ReadPacketBytes(packetBuffer) + // have a packetsize, now reading the whole packet + packetBuffer := make([]byte, packetSize) + connection.Read(packetBuffer) - isvalid, _ := IsValidPacket(packet) - if isvalid { - return packet - } + packet, err := ReadPacketBytes(packetBuffer) + if err != nil { + return Packet{}, err + } - break + isvalid, msg := IsValidPacket(packet) + if isvalid { + return packet, nil } - return Packet{} + return Packet{}, fmt.Errorf("received an invalid packet. Reason: %s", msg) } diff --git a/server/server.go b/server/server.go index 64d93f4..d7c1a35 100644 --- a/server/server.go +++ b/server/server.go @@ -5,6 +5,7 @@ import ( "io" "net" "net/http" + "os" "github.com/Unbewohnte/FTU/protocol" ) @@ -19,7 +20,7 @@ func GetLocalIP() (string, error) { localAddr := conn.LocalAddr().(*net.UDPAddr) - return string(localAddr.IP), nil + return localAddr.IP.String(), nil } // gets a remote ip. Borrowed from StackOverflow, thank you, whoever I brought it from @@ -100,15 +101,14 @@ func (s *Server) Disconnect() { } // Accepts one connection -func (s *Server) WaitForConnection() error { +func (s *Server) WaitForConnection() { connection, err := s.Listener.Accept() if err != nil { - return fmt.Errorf("could not accept a connection: %s", err) + fmt.Printf("Could not accept a connection: %s", err) + os.Exit(-1) } s.Connection = connection fmt.Println("New connection from ", s.Connection.RemoteAddr()) - - return nil } // Closes the listener. Used only when there is still no connection from `AcceptConnections` @@ -174,10 +174,11 @@ func (s *Server) SendPiece() error { // Listens in an endless loop; reads incoming packages and puts them into channel func (s *Server) ReceivePackets() { for { - incomingPacket := protocol.ReadFromConn(s.Connection) - isvalid, _ := protocol.IsValidPacket(incomingPacket) - if !isvalid { - continue + incomingPacket, err := protocol.ReadFromConn(s.Connection) + if err != nil { + // in current implementation there is no way to receive a working file even if only one packet is missing + fmt.Printf("Error reading a packet: %s\nExiting...", err) + os.Exit(-1) } s.IncomingPackets <- incomingPacket } @@ -226,7 +227,8 @@ func (s *Server) MainLoop() { } err := s.SendPiece() if err != nil { - fmt.Println(err) + fmt.Printf("Could not send a piece of file: %s", err) + os.Exit(-1) } case protocol.HeaderDisconnecting: