Buffer for TCP data

Question

For an assignment (not school), I was supposed to make a "web browser" using Scapy.

While storing the response, I initially thought of

# got an array of packets using sniff() named responses
ans = ""
for pack in responses:
 ans += pack[Raw].load

But that clearly doesn't work if one of the ACKs gets lost or with any of that kind of edge cases where I receive more than one of the same packet or where I receive them in the wrong order. So I wrote a "buffer" class to help handle that.

Code:

class MessageBuffer:
 """
 Used to store a buffer or message
 """
 def __init__(self, init_key):
 """
 Creates a MessageBuffer object that will hold a buffer
 containing all the message that will be received.
 :param init_key: Initial sequence number
 """
 self._buffer = []
 self._init_key = init_key
 self.unsafe = False # Can be set to be able to convert incomplete buffer
 def __setitem__(self, key, value):
 """
 Adds a message into the buffer, automatically extending the storage if needed.
 :param key: the starting point
 :param value: the string to insert
 """
 key -= self._init_key
 if key < 0:
 raise KeyError("Key cannot be less than initial key")
 if key > len(self._buffer): # Missing values
 while key > len(self._buffer):
 self._buffer.append("") # Extend till Ok
 self._buffer.extend(value) # Add all string at the end.
 else:
 try:
 while value:
 self._buffer[key] = value[0]
 key += 1
 value = value[1:]
 except IndexError: # End of self._buffer
 self._buffer.extend(value)
 def is_complete(self):
 """
 :return: True if buffer is complete, otherwise False
 """
 return "" not in self._buffer
 def __str__(self):
 """
 Converts the buffer into a string.
 """
 if not self.unsafe and not self.is_complete():
 raise Exception("Buffer isn't complete")
 return "".join(self._buffer)

I don't have particular concerns, but maybe I have some kind of bad practice or something like that in the code? Also, no usage of the class but in pseudo-python-code (since copying the whole code is pretty long and not what I'm asking about)

def handler_for_packet(packet):
 buffer[packet[TCP].seq] = packet[Raw].load
 if buffer.is_complete():
 send_ack_for_packet(packet)

• Amit Gold: @200_success quick question - why didn't I receive a notification about the edit?
• 200_success: I'm not sure, but I suspect that tag-only edits might be handled differently.

Answer 1

I'd do a complete re-write of __setitem__. The rest of you class is just design decisions. And I don't have a solid alternate to it.

But __setitem__ is bad:

• while loops are normally un-idiomatic as you can better describe the loop with a for loop.
• You've abused error handling, as exceptions should be somewhat exceptional, not for when you don't use for. But if you change the while value loop to a for loop, extending would be simple.
• Slicing an array to reduce its size multiple times is slow, \$O(n^2)\$ slow, and so you shouldn't do it. And using list.pop(0) is as bad.
• Don't raise a KeyError on an item that's using indexes. If you were using a dictionary it'd make sense, but you're not.

Instead you should extend the buffer to be as long as needed, key + len(value). And so you'd only need to add key + len(value) - len(self._buffer) amount of spaces.

After this you just need to insert the data into the buffer, and as we've guaranteed it'll be long enough, we can just set a slice of the buffer. Where the start is key and the end is key + len(value).

And so it can be:

def __setitem__(self, index, value):
 index -= self._init_index
 if index < 0:
 raise IndexError("Index cannot be less than initial index.")
 end = index + len(value)
 buff = self._buffer
 buff.extend(" " * (end - len(self._buffer)))
 buff[index:end] = value

This is better than yours as most of the processing is now in C, rather than Python, and so it should be faster. It's also \$O(n)\$ rather than \$O(n ^ 2)\$. But not only that, it's easier to read.