meta data for this page
  •  

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revision Previous revision
Last revision Both sides next revision
courses:ct30a5000:assignment2notes [2012/10/16 21:29]
julaakko [Test your application!]
courses:ct30a5000:assignment2notes [2012/10/17 13:29]
julaakko [Comments]
Line 8: Line 8:
 | 0312399 | Ok division of code. Partial send should handle the possible error in sending, same for reading. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Nice wey of tokenizing user input. With IPv6 support you should use struct sockaddr_storage instead of sockaddr_in → IPv6 support is insufficient. Server message handling can be blocked if client sends only 4 bytes → server keeps awaiting for more, consider implementing this completely with select and read what you can then process packets after they are fully received. The length of payload in protocol packets is not properly checked using the length-header-nickname. Invalid binary name. Only IPv6 addresses work with client. If client gives plain “/​connect” without any parameters it gets no response but when given next time with a message server reports that nickname is in use. Server does not seem to handle all premature disconnections since in previously mentioned situation the server replies that nickname is in use. If nickname is too long the message is sent to others. | | 0312399 | Ok division of code. Partial send should handle the possible error in sending, same for reading. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Nice wey of tokenizing user input. With IPv6 support you should use struct sockaddr_storage instead of sockaddr_in → IPv6 support is insufficient. Server message handling can be blocked if client sends only 4 bytes → server keeps awaiting for more, consider implementing this completely with select and read what you can then process packets after they are fully received. The length of payload in protocol packets is not properly checked using the length-header-nickname. Invalid binary name. Only IPv6 addresses work with client. If client gives plain “/​connect” without any parameters it gets no response but when given next time with a message server reports that nickname is in use. Server does not seem to handle all premature disconnections since in previously mentioned situation the server replies that nickname is in use. If nickname is too long the message is sent to others. |
 | 0316023 | Ok division of code. Partial sends are not handled, remember to check how many bytes have been sent and is there need to send the rest. Partial receiving is not handled – the packet length Is not checked properly, you should always verify that we've received a full packet before processing and if not, receive the rest. The length is utilized well in packet parsing – good. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Client can connect to server when already taken nick is replaced with new – nice. | | 0316023 | Ok division of code. Partial sends are not handled, remember to check how many bytes have been sent and is there need to send the rest. Partial receiving is not handled – the packet length Is not checked properly, you should always verify that we've received a full packet before processing and if not, receive the rest. The length is utilized well in packet parsing – good. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Client can connect to server when already taken nick is replaced with new – nice. |
-| 0342246 | Adding more comments to code wouldn'​t hurt in the future. Good division of code. Nice way of handling partial data. Might be a good idea to differentiate between select() error (-1) and timeout (0). Quitting the application when less than header size is read is not really a good idea and with MSG_WAITALL the application might be vulnerable for DoS attacks – read what you can and if it is not enough try to read more next time data is detected on socket. Otherwise partial data reading/​writing is done well but errors with recv() and send() (-1) should be handled too. The nickname buffer is not padded with NULLs since it is not initialized at any point. Invalid binary name. Duplicate nicknames are not checked. Client can be crashed with invalid input (letters and numbers as port)+| 0342246 | Adding more comments to code wouldn'​t hurt in the future. Good division of code. Nice way of handling partial data. Might be a good idea to differentiate between select() error (-1) and timeout (0). Quitting the application when less than header size is read is not really a good idea and with MSG_WAITALL the application might be vulnerable for DoS attacks – read what you can and if it is not enough try to read more next time data is detected on socket. Otherwise partial data reading/​writing is done well but errors with recv() and send() (-1) should be handled too. The nickname buffer is not padded with NULLs since it is not initialized at any point. Invalid binary name. Duplicate nicknames are not checked. Client can be crashed with invalid input (letters and numbers as port) |
 | 0342327 | Ok division of code but reading some guides about code indentation wouldn'​t hurt – hard to read this stuff. More comments would ease the reading. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Remember to always check that correct amount of data is sent. Partial sending or receiving is not handled. Do not expect that you have more data awaiting in the network buffer – your approach will open possibilities for easy DoS attacks as server would block when only header is sent.  Lots of valgrind errors. Nickname change from client leaves a newline marker into nickname and server does not initialize the nickname list that is sent, each request increases the list that is sent. Client leaks memory. | | 0342327 | Ok division of code but reading some guides about code indentation wouldn'​t hurt – hard to read this stuff. More comments would ease the reading. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Remember to always check that correct amount of data is sent. Partial sending or receiving is not handled. Do not expect that you have more data awaiting in the network buffer – your approach will open possibilities for easy DoS attacks as server would block when only header is sent.  Lots of valgrind errors. Nickname change from client leaves a newline marker into nickname and server does not initialize the nickname list that is sent, each request increases the list that is sent. Client leaks memory. |
 | 0358454 | Good division of code.  For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Partial sending or receiving not handled, the approach using loop to peek the length will introduce problems at the second round since it starts writing to buf[0] at every round. You should always check that is the packet fully received before processing – also processing could be done after the reading of incoming data: separating these functions might bring flexibility. The packet length is converted to network order with htons() instead of htonl() → the result is 16bit instead of 32bit, this works in your application but does not work with others. You should make sure that nicknames have terminating NULL at the end especially when the nickname list is sent (nicks must be separated with NULLs as stated in specification). Client leaks memory. Client does not report connection error if e.g. port is wrong. | | 0358454 | Good division of code.  For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Partial sending or receiving not handled, the approach using loop to peek the length will introduce problems at the second round since it starts writing to buf[0] at every round. You should always check that is the packet fully received before processing – also processing could be done after the reading of incoming data: separating these functions might bring flexibility. The packet length is converted to network order with htons() instead of htonl() → the result is 16bit instead of 32bit, this works in your application but does not work with others. You should make sure that nicknames have terminating NULL at the end especially when the nickname list is sent (nicks must be separated with NULLs as stated in specification). Client leaks memory. Client does not report connection error if e.g. port is wrong. |