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)

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.