google / google/error-prone

ByteBufferBackingArray should recognize hasArray() guards

Open
#5,241 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
7.2k
Forks
820
Avg merge
5h 9m
Merged PRs (30d)
50

Description

**Background:**
In https://bugs.webrtc.org/42234355, I'm trying to remove `@SuppressWarnings("ByteBufferBackingArray")` and add proper ByteBuffer backing array checks.

The following code seems the correct pattern for safely accessing a ByteBuffer's backing array:
```java
final int frameSize = 100;
final ByteBuffer buffer = ByteBuffer.allocateDirect(frameSize);
if (buffer.hasArray()) {
buffer.array();
} else {
final byte[] array = new byte[frameSize];
buffer.get(array);
}
```

However, Error Prone's `ByteBufferBackingArray` checker incorrectly flags this as unsafe.

**Current behavior:**
Upon reviewing `ByteBufferBackingArray.java`, the checker currently only accepts:
- Calls to `ByteBuffer.arrayOffset()`
- Buffers created with `ByteBuffer.wrap()` or `ByteBuffer.allocate()`

This is insufficient for the recommended `hasArray()` guard pattern.

**Expected behavior:**
The checker should accept `array()` calls when properly guarded by conditional `hasArray()` checks.

**Should NOT warn:**
```java
final ByteBuffer buffer = ByteBuffer.allocateDirect(100);
if (buffer.hasArray()) {
buffer.array(); // Safe - guarded by hasArray()
}
```

**Should warn:**
```java
final ByteBuffer buffer = ByteBuffer.allocateDirect(100);
buffer.hasArray(); // Just calling hasArray() without using the result
buffer.array(); // Unsafe - no conditional protection
```

**Suggestion:**
Enhance `ByteBufferBackingArray` to recognize when `array()` calls are control-flow guarded by `hasArray()` conditions, treating them as safe usage patterns.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.