make sure the command length header data be 4 bytes

[jwenjian]

Thank you for contributing to naz.
Every contribution to naz is important.

Contributor offers to license certain software (a “Contribution” or multiple
“Contributions”) to naz, and naz agrees to accept said Contributions,
under the terms of the MIT License.
Contributor understands and agrees that naz shall have the irrevocable and perpetual right to make
and distribute copies of any Contribution, as well as to create and distribute collective works and
derivative works of any Contribution, under the MIT License.

Now,

What(What have you changed?)

• when receiving data from SMSC, make sure the command_length_header_data be 4 bytes

Git diff:

diff --git a/naz/client.py b/naz/client.py
index 0a83916..fe8f3d4 100644
--- a/naz/client.py
+++ b/naz/client.py
@@ -1852,6 +1852,12 @@ class Client:
                 # `client.reader` and `client.writer` should not have timeouts since they are non-blocking
                 # https://github.com/komuw/naz/issues/116
                 command_length_header_data = await self.reader.read(4)
+
+                # make sure the header data be 4 bytes
+                while len(command_length_header_data) != 4:
+                    more_bytes = await self.reader.read(4 - len(command_length_header_data))
+                    command_length_header_data = command_length_header_data + more_bytes
+
             except (
                 ConnectionError,
                 TimeoutError,

Why(Why did you change it?)

• self.reader.read(4) will not ensure the bytes length be 4, if so, the struct.unpack(">I", command_length_header_data) will throw an exception with message: unpack requires a buffer of 4 bytes

I faced the issue when I am doing a stress testing with SMSC, in which case, the naz client will receive larget amount of MO message from SMSC, after running correctly for about 5 minutes, the exception unpack requires a buffer of 4 bytes occurred.
The naz client only read 1 byte from SMSC socket connection even there has more data after.

So I made this update, and the stress test goes well, for now.

References:

[codecov-io]

Codecov Report

Merging #153 into master will decrease coverage by 0.12%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master     #153      +/-   ##
==========================================
- Coverage   86.36%   86.23%   -0.13%     
==========================================
  Files          14       14              
  Lines        1305     1308       +3     
==========================================
+ Hits         1127     1128       +1     
- Misses        178      180       +2

Impacted Files	Coverage Δ
naz/client.py	80.18% <33.33%> (-0.22%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29332b3...905ce81. Read the comment docs.

[komuw]

hi @jwenjian

thanks, for this. I'll try and do a proper review later on.
(i) what version of naz are you currently using?
(ii) is this an issue you have experienced while naz or are you been proactive in defense

[komuw]

@jwenjian I wonder if using await self.reader.readexactly(4)[1] would be better?
although that raises IncompleteReadError[2] which we would have to handle somehow.

1. https://docs.python.org/3.6/library/asyncio-stream.html#asyncio.StreamReader.readexactly
2. https://docs.python.org/3.6/library/asyncio-stream.html#asyncio.IncompleteReadError

[komuw]

Also relevant[3].

I do not know which is better. Let me know your thoughts

3. https://stackoverflow.com/questions/28452724/method-asyncio-streams-streamreader-readbyte-count-reads-too-few-bytes

[jwenjian]

The readexactly method is better if we have a proper way to handle the IncompleteReadError

log an error and ignore it or raise it?

[jwenjian]

It seems there is no timeout in the read*() functions?

So if the readexactly(4) raises an error, it means that there is no more data and the read partial bytes(length must be less than 4) is not part of an valid SMPP PDU, so I suggest to abandon it then continue the while loop.

[jwenjian]

In my opinion, like the way you handle the message body in Line 1900, it should works fine to the headers.

also don't know which is better, so maybe we can use the way I do in this pr util we find out a more efficient way.

[komuw]

thanks for your input. I think we can start with this and improve if/when it becomes neccessary

I'll think about it some more.

[jwenjian]

@komuw

1. naz version is 0.6.5 which I installed by execute pip install naz
2. it's a issue I experienced when I use naz as a SMPP Client to test my SMSC with a large amount of MO messages