search

Tanganelli / CoAPthon

CoAPthon is a python library to the CoAP protocol aligned with the RFC
MIT License
223 stars 131 forks source link

BlockLayer receive_response bug #87

Open MainReturnZero opened 7 years ago

MainReturnZero commented 7 years ago

Line 143-147: it should be item.m instead of m. This issue will occur when using put or post transfer a large file or large payload(>1024byte). This bug is introduced at this commit: https://github.com/Tanganelli/CoAPthon/commit/3b80769f183367ed39283e20573718e55b7468df#diff-956c81d955e4d02e1bbae6100008141f

You should not rely on server will response with block1 is None to stop blockwise transfer. In the standard, it says: A Block1 Option in control usage in a response (e.g., a 2.xx response for a PUT or POST request):

  *  The NUM field of the Block1 Option indicates what block number
     is being acknowledged.

So, when responding to the last block, the server should not delete Block1 field.

Tanganelli commented 6 years ago

Hi, sorry but I don't understand which source file has the bug on line 143-147...

sunbit commented 6 years ago

Altough i cannot find either those lines refered, i have the same problem, and it's just what that user says. the block1 response of the last block from the server (Using californium a server) sets the M flag to 0, but the block is still there.

Looking at the code, in the blocklayer.py, receive_response method https://github.com/Tanganelli/CoAPthon/blob/18478edd869cc3de2e2e0bf82c09a1814bb90504/coapthon/layers/blocklayer.py#L122

It assumes that the block won't be there. I changed that line into, without removing the None check:

blockwise_finished = transaction.response.block1 is None or transaction.response.block1[1] == 0
    if key_token in self._block1_sent and (not blockwise_finished):

And with that the blockwise transfers ends correctly

Tanganelli commented 6 years ago

@sunbit Could you create a pull request so I will merge it into the code?

Thanks Giacomo