fuzz: add target for FixedSizeCache - #547
Conversation
📝 WalkthroughWalkthroughAdded a .NET 10 SharpFuzz executable for ChangesFixedSizeCache fuzzing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Program
participant SharpFuzz
participant FixedSizeCacheFuzz
participant FixedSizeCache
Program->>Program: inspect command-line arguments
Program->>SharpFuzz: start out-of-process fuzzing
SharpFuzz->>FixedSizeCacheFuzz: provide fuzz input
FixedSizeCacheFuzz->>FixedSizeCache: add, contains, and remove keys
FixedSizeCache-->>FixedSizeCacheFuzz: cache operation results
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@NBXplorer.Fuzz/FixedSizeCacheFuzz.cs`:
- Around line 189-205: Remove the ArgumentNullException and
ArgumentOutOfRangeException catch blocks from the Fuzzer.OutOfProcess callback
so exceptions from FixedSizeCacheFuzz.Run propagate to SharpFuzz and failing
inputs are recorded.
- Around line 115-155: Change the cache declaration to use FuzzableKey for both
key and value, with an identity key selector. In the operation switch, pass the
generated key directly to Add, Contains, and Remove instead of constructing and
using the string value, so FuzzableKey.GetHashCode() and Data drive cache
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f9c5618-c0f5-4d21-8b57-dde54a903a2d
📒 Files selected for processing (2)
NBXplorer.Fuzz/FixedSizeCacheFuzz.csNBXplorer.Fuzz/NBXplorer.Fuzz.csproj
| var cache = new FixedSizeCache<string, FuzzableKey>( | ||
| cacheSize, | ||
| s => new FuzzableKey(StableHash(s), Encoding.UTF8.GetBytes(s ?? "")) | ||
| ); | ||
|
|
||
| for (int i = 0; i < numOps; i++) | ||
| { | ||
| if (offset >= data.Length) | ||
| break; | ||
|
|
||
| int op = ReadByte() % 3; | ||
|
|
||
| FuzzableKey key; | ||
| bool useEdgeCase = (ReadByte() % 4) == 0; | ||
|
|
||
| if (useEdgeCase) | ||
| { | ||
| int idx = ReadByte() % EdgeCaseHashCodes.Length; | ||
| int hc = EdgeCaseHashCodes[idx]; | ||
| var keyData = ReadBytes(ReadByte() % 32); | ||
| key = new FuzzableKey(hc, keyData); | ||
| } | ||
| else | ||
| { | ||
| int hc = ReadInt32(); | ||
| var keyData = ReadBytes(ReadByte() % 28); | ||
| key = new FuzzableKey(hc, keyData); | ||
| } | ||
|
|
||
| string value = $"fuzz_{i}_{key.HashCode}"; | ||
|
|
||
| switch (op) | ||
| { | ||
| case 0: | ||
| cache.Add(value); | ||
| break; | ||
| case 1: | ||
| cache.Contains(value); | ||
| break; | ||
| case 2: | ||
| cache.Remove(value); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use FuzzableKey as the cache value and key.
The cache derives its key from StableHash(value). Lines 149, 152, and 155 pass a string value. Therefore, FuzzableKey.GetHashCode() does not control the cache bucket. key.Data also has no effect on the cache operation.
Use FixedSizeCache<FuzzableKey, FuzzableKey> with an identity key selector. Pass key to Add, Contains, and Remove.
Proposed fix
- var cache = new FixedSizeCache<string, FuzzableKey>(
- cacheSize,
- s => new FuzzableKey(StableHash(s), Encoding.UTF8.GetBytes(s ?? ""))
- );
+ var cache = new FixedSizeCache<FuzzableKey, FuzzableKey>(
+ cacheSize,
+ key => key
+ );
...
- string value = $"fuzz_{i}_{key.HashCode}";
-
switch (op)
{
case 0:
- cache.Add(value);
+ cache.Add(key);
break;
case 1:
- cache.Contains(value);
+ cache.Contains(key);
break;
case 2:
- cache.Remove(value);
+ cache.Remove(key);
break;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var cache = new FixedSizeCache<string, FuzzableKey>( | |
| cacheSize, | |
| s => new FuzzableKey(StableHash(s), Encoding.UTF8.GetBytes(s ?? "")) | |
| ); | |
| for (int i = 0; i < numOps; i++) | |
| { | |
| if (offset >= data.Length) | |
| break; | |
| int op = ReadByte() % 3; | |
| FuzzableKey key; | |
| bool useEdgeCase = (ReadByte() % 4) == 0; | |
| if (useEdgeCase) | |
| { | |
| int idx = ReadByte() % EdgeCaseHashCodes.Length; | |
| int hc = EdgeCaseHashCodes[idx]; | |
| var keyData = ReadBytes(ReadByte() % 32); | |
| key = new FuzzableKey(hc, keyData); | |
| } | |
| else | |
| { | |
| int hc = ReadInt32(); | |
| var keyData = ReadBytes(ReadByte() % 28); | |
| key = new FuzzableKey(hc, keyData); | |
| } | |
| string value = $"fuzz_{i}_{key.HashCode}"; | |
| switch (op) | |
| { | |
| case 0: | |
| cache.Add(value); | |
| break; | |
| case 1: | |
| cache.Contains(value); | |
| break; | |
| case 2: | |
| cache.Remove(value); | |
| var cache = new FixedSizeCache<FuzzableKey, FuzzableKey>( | |
| cacheSize, | |
| key => key | |
| ); | |
| for (int i = 0; i < numOps; i++) | |
| { | |
| if (offset >= data.Length) | |
| break; | |
| int op = ReadByte() % 3; | |
| FuzzableKey key; | |
| bool useEdgeCase = (ReadByte() % 4) == 0; | |
| if (useEdgeCase) | |
| { | |
| int idx = ReadByte() % EdgeCaseHashCodes.Length; | |
| int hc = EdgeCaseHashCodes[idx]; | |
| var keyData = ReadBytes(ReadByte() % 32); | |
| key = new FuzzableKey(hc, keyData); | |
| } | |
| else | |
| { | |
| int hc = ReadInt32(); | |
| var keyData = ReadBytes(ReadByte() % 28); | |
| key = new FuzzableKey(hc, keyData); | |
| } | |
| switch (op) | |
| { | |
| case 0: | |
| cache.Add(key); | |
| break; | |
| case 1: | |
| cache.Contains(key); | |
| break; | |
| case 2: | |
| cache.Remove(key); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@NBXplorer.Fuzz/FixedSizeCacheFuzz.cs` around lines 115 - 155, Change the
cache declaration to use FuzzableKey for both key and value, with an identity
key selector. In the operation switch, pass the generated key directly to Add,
Contains, and Remove instead of constructing and using the string value, so
FuzzableKey.GetHashCode() and Data drive cache behavior.
| Fuzzer.OutOfProcess.Run(stream => | ||
| { | ||
| try | ||
| { | ||
| using var memoryStream = new MemoryStream(); | ||
| stream.CopyTo(memoryStream); | ||
| FixedSizeCacheFuzz.Run(memoryStream.ToArray()); | ||
| } | ||
| catch (ArgumentNullException) | ||
| { | ||
| // Expected for null inputs | ||
| } | ||
| catch (ArgumentOutOfRangeException) | ||
| { | ||
| // Expected for edge-case parameters | ||
| } | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not suppress exceptions from the fuzz target.
This target creates a positive cache size and passes non-null operation values. ArgumentNullException and ArgumentOutOfRangeException are not expected outcomes for these inputs. The catch blocks hide cache regressions from SharpFuzz.
Remove these handlers so that SharpFuzz records the failing input.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@NBXplorer.Fuzz/FixedSizeCacheFuzz.cs` around lines 189 - 205, Remove the
ArgumentNullException and ArgumentOutOfRangeException catch blocks from the
Fuzzer.OutOfProcess callback so exceptions from FixedSizeCacheFuzz.Run propagate
to SharpFuzz and failing inputs are recorded.
1 participant
NBXplorer has no fuzz coverage at all so had to write this from scratch. I'm not a C#/.NET expert so did a quick search about fuzz testing tools for it and noticed that SharpFuzz would be the most suitable for it. The idea of this initial target is to exercise the FixedSizeCache with a fuzzable key: a key whose GetHashCode() is directly controlled by the fuzzer.