feat: add File Store, implement Tag, Resolve, Exists#317
feat: add File Store, implement Tag, Resolve, Exists#317wangxiaoxuan273 wants to merge 15 commits into
Conversation
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #317 +/- ##
==========================================
+ Coverage 91.45% 91.51% +0.06%
==========================================
Files 61 66 +5
Lines 2586 2818 +232
Branches 345 369 +24
==========================================
+ Hits 2365 2579 +214
- Misses 137 152 +15
- Partials 84 87 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a file-based storage implementation for ORAS .NET, adding the Store class that provides content-addressable storage backed by the file system. The implementation includes methods for adding files, tagging descriptors, resolving tags, and checking content existence.
Key changes:
- Added
Storeclass with Add, Tag, Resolve, Exists, and Close operations for file-based storage - Introduced four new exception types to handle store-specific error conditions
- Added
AnnotationTitleconstant toDescriptorfor OCI image title annotation support
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
src/OrasProject.Oras/Content/File/Store.cs |
Core file store implementation with named content tracking, fallback storage, and thread-safe operations |
src/OrasProject.Oras/Oci/Descriptor.cs |
Added AnnotationTitle constant for OCI-compliant image title annotations |
src/OrasProject.Oras/Exceptions/DuplicateNameException.cs |
New exception thrown when attempting to add a duplicate name to the store |
src/OrasProject.Oras/Exceptions/MissingNameException.cs |
New exception thrown when a required name parameter is missing |
src/OrasProject.Oras/Exceptions/MissingReferenceException.cs |
New exception thrown when a required reference parameter is missing |
src/OrasProject.Oras/Exceptions/StoreClosedException.cs |
New exception thrown when operations are attempted on a closed store |
tests/OrasProject.Oras.Tests/Content/File/StoreTest.cs |
Comprehensive test suite covering store lifecycle, operations, validation, and error handling |
tests/OrasProject.Oras.Tests/Exceptions/ExceptionTest.cs |
Tests verifying the new exception types can be instantiated and thrown correctly |
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
cshung
left a comment
There was a problem hiding this comment.
I've identified a TOCTOU (Time-of-Check to Time-of-Use) race condition that needs to be addressed before approval.
| } | ||
|
|
||
| // if the target has name, check if the name exists. | ||
| if (target.Annotations != null && target.Annotations.TryGetValue(Descriptor.AnnotationTitle, out var name)) |
There was a problem hiding this comment.
The current IsClosedSet() check might not fully address TOCTOU (Time-of-Check to Time-of-Use) race conditions.
Consider a situation where:
- Thread 1 is checking whether an element exists in the store (
ExistsAsync) - Thread 2 is simultaneously closing the store (
Close())
Expected outcomes:
- Thread 1 completes before close → returns result successfully
- Thread 2 closes first → Thread 1 receives
StoreClosedException
However, if operations interleave as:
- Thread 1 calls
IsClosedSet()→ passes (store not closed yet) - Thread 2 calls
Close()→ deletes files via_tmpFiles - Thread 1 proceeds to file access → file access exception
This is the classic TOCTOU vulnerability where the time-of-check ≠ time-of-use. The IsClosedSet() flag alone cannot prevent this race. Store access and closing operations need proper mutual exclusion (locks, semaphores, etc.).
There was a problem hiding this comment.
Thanks for the comment Andrew. I have added a semaphore against Close(), so that when other operations are in process, Close() cannot be called, and when Close() is in process, other operations cannot be called. Can you take a look and see if there's any possible improvement?
| /// </summary> | ||
| private class NameStatus | ||
| { | ||
| public SemaphoreSlim Semaphore { get; } = new(1, 1); |
There was a problem hiding this comment.
The SemaphoreSlim created here is never disposed. When Store.Dispose() is called, these semaphores remain in memory and could lead to memory leak
There was a problem hiding this comment.
Thanks for catching this. Made NameStatus to implement IDisposable, and called Dispose when calling Store.Close().
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
|
superseded by #328 |
Part of #37
Added File Store to oras-dotnet. Implemented Tag, Resolve, Exists, Add and Close methods. Adding a directory is not yet supported and will be added in a future PR.