Repository: apple/swift-nio
File: Sources/NIOCore/ByteBuffer-core.swift (Swift)

Code before:
                    byteCount: self.readableBytes
                )
            let indexShift = self._readerIndex
            self._moveReaderIndex(to: 0)
            self._moveWriterIndex(to: self._writerIndex - indexShift)
        } else {
            self._copyStorageAndRebase(extraCapacity: 0, resetIndices: true)
        }
        return true
    }
    
    /// The `ByteBuffer` will successfully be shrunk if the requested capacity is less than the current capacity, 
    /// and the requested capacity is more than the number of readable bytes in the buffer.
    /// If either condition is not true, the buffer will not be shrunk.
    ///
    /// - Parameter desiredCapacity: The desired capacity for the buffers capacity to be shrunken to
    /// - Returns: Bool indicating whether the buffer capacity has been shrunk to the desiredCapacity.
    @inlinable
    @discardableResult public mutating func shrinkBufferCapacity(to desiredCapacity: Int) -> Bool {
        let desiredCapacity = ByteBuffer.addPaddingTo(desiredCapacity)
        guard desiredCapacity < capacity, desiredCapacity > readableBytes else {
            return false
        }
        
        let shrunkenCapacity = _toCapacity(desiredCapacity)
        self._storage = self._storage.reallocSlice(self._slice.lowerBound..<self._slice.lowerBound + shrunkenCapacity, capacity: shrunkenCapacity)
        self._slice = self._storage.fullSlice

        return true
    }
    
    /// Returns size of capacity with optimal padding
    /// - Parameter initialCapacity: Capacity that needs expansion with padding
    /// - Returns: Capacity with calculated padding 
    @inlinable
    static func addPaddingTo(_ initialCapacity: Int) -> Int {
        initialCapacity == 0 ? 0 : initialCapacity.nextPowerOf2()
    }

    /// The reader index or the number of bytes previously read from this `ByteBuffer`. `readerIndex` is `0` for a
    /// newly allocated `ByteBuffer`.
    @inlinable
    public var readerIndex: Int {
        Int(self._readerIndex)
    }

    /// The write index or the number of bytes previously written to this `ByteBuffer`. `writerIndex` is `0` for a
    /// newly allocated `ByteBuffer`.
    @inlinable
    public var writerIndex: Int {
        Int(self._writerIndex)

Review comment (suggestion):
I think this reads potentially unwritten bytes which is UB.

Let's say we have an old ByteBuffer with `capacity: 8` in this state

```
slice.lowerBound (0)
| readerIndex (2)
| | writerIndex (4)
| | |   slice.upperBound (8, one past the end of the slice)
| | |   |
v v v   v
uuWWuuuu
```

If we now call `shrinkBufferCapacity(to: 4)` which will work, it's more than `self.readableBytes` but less than `self.capacity`, then we'll allocate a new storage of size 4 and copy `uuWW` in using `memcpy` in `reallocSlice`, that's UB.

where:

- `W`: is a byte written to (and readable)
- `u`: is a byte never written

We could create such a buffer as

```
var buf = ByteBuffer(capacity: 8)
buf.setWriterIndex(to: 2)
buf.setReaderIndex(to: 2)
buf.writeString("WW")
buf.shrinkBufferCapacity(to: 4)
XCTAssertEqual("WW", String(buffer: buf))
```

if this is run through Valgrind, I would expect valgrind to complain that we read bytes that were never written.


Code after:
            self._storage.bytes.advanced(by: Int(self._slice.lowerBound))
                .copyMemory(
                    from: self._storage.bytes.advanced(by: Int(self._slice.lowerBound + self._readerIndex)),
                    byteCount: self.readableBytes
                )
            let indexShift = self._readerIndex
            self._moveReaderIndex(to: 0)
            self._moveWriterIndex(to: self._writerIndex - indexShift)
        } else {
            self._copyStorageAndRebase(extraCapacity: 0, resetIndices: true)
        }
        return true
    }

    /// The `ByteBuffer` will successfully be shrunk if the requested capacity is less than the current capacity,
    /// and the requested capacity is greater than or equal to the number of readable bytes in the buffer.
    /// If either condition is not true, the buffer will not be shrunk.
    ///
    /// - Parameter desiredCapacity: The desired capacity for the buffers capacity to be shrunken to
    /// - Returns: Bool indicating whether the buffer capacity has been shrunk to the desiredCapacity.
    @inlinable
    @discardableResult public mutating func shrinkBufferCapacity(to desiredCapacity: Int) -> Bool {
        let desiredCapacity = ByteBuffer.roundUpToUsableCapacity(_toCapacity(desiredCapacity))
        guard desiredCapacity < self.capacity, desiredCapacity >= self.readableBytes else {
            return false
        }
        self._copyStorageAndRebase(capacity: desiredCapacity, resetIndices: true)
        return true
    }

    /// Returns size of capacity with optimal padding
    /// - Parameter initialCapacity: Capacity that needs expansion with padding
    /// - Returns: Capacity with calculated padding
    @inlinable
    static func roundUpToUsableCapacity(_ initialCapacity: UInt32) -> UInt32 {
        initialCapacity == 0 ? 0 : initialCapacity.nextPowerOf2ClampedToMax()
    }

    /// The reader index or the number of bytes previously read from this `ByteBuffer`. `readerIndex` is `0` for a
    /// newly allocated `ByteBuffer`.
    @inlinable
    public var readerIndex: Int {
        Int(self._readerIndex)
    }

    /// The write index or the number of bytes previously written to this `ByteBuffer`. `writerIndex` is `0` for a
    /// newly allocated `ByteBuffer`.
    @inlinable
    public var writerIndex: Int {
        Int(self._writerIndex)
    }