Fix double incrementing of the packet number

[deepankar-j]

This MR fixes an issue where the packet number field of the packet header was incorrectly incremented twice. This resulted in the packet number fo a single LOGIN7 packet being 2, rather 1.

This violated the TDS protocol definition. Furthermore, firewall rules that restrict traffic based on the TDS protocol filtered out such violating TDS packets. Also, it resulted in tools such as Wireshark not being able to properly parse out the TDS header.

Example

Here's an example of two LOGIN7 packets. The packet on the left was generated with SHA 748b2f4. The packet on the right was generated using SHA fe62754. As you can see, the packet number is wrong in the left screenshot.

Issue Origination

I believe the issue was introduced in SHA 748b2f4.

Unit Tests

I've added unit test for boundary testing the encode_packets/3 function.

[coveralls]

Coverage remained the same at 73.129% when pulling 2bda5fb on deepankar-j:fix-packet-number into 11e2f91 on livehelpnow:master.

[deepankar-j]

@mjaric: Could you please review this PR? It would be great to get this change incorporated into the mainline so that we can move off of our fork of tds.

[mjaric]

Thanks!

[mjaric]

Just one question, if first packet is zero, would firewall complain too? I'm not sure who could send more than 254 packet as single message, but after 255 counter must be reset. I know that server will return response, but what will happen with firewall I'm not sure

[deepankar-j]

According to the latest TDS spec (v30), starting with an index of 1 is acceptable and what .NET uses.

I suspect that it might vary between firewalls whether they properly support a PacketID of 0. However, starting with an ID of 1 is probably the safest bet.

2.2.3.1.5 PacketID
PacketID is used for numbering message packets that contain data in addition to the packet header.
PacketID is a 1-byte, unsigned char. Each time packet data is sent, the value of PacketID is
incremented by 1, modulo 256.<7> This allows the receiver to track the sequence of TDS packets for
a given message. This value is currently ignored.

Footnote:

<7> Section 2.2.3.1.5: Depending on the message type and provider, such as Microsoft SQL Server Native Client or Microsoft .NET Framework Data Provider for SQL Server, PacketID values start with either 0 or 1, which is an implementation choice. The .NET Framework Data Provider for SQL Server uses 1.