fix(vine,filesystem): remove unconditional RegisterOperation leak; fix WriteFile byte-array decode and Stat path ordering#72
Merged
Conversation
…-array decode and Stat empty-path ordering Closes #63, #65 Issue #63 (ActiveOperations leak): - Remove unconditional RegisterOperation call from process_cocoon_request. No handler ever called UnregisterOperation, so the map grew without bound. Operations that need cancellation must opt-in explicitly from their handler. - cancel_operation already removes the entry after token.cancel(); no change needed there. - Remove ValidateRequest checks for "::" and "../" in method names per issue comment (path traversal is in params, not method names; these checks block legitimate callers). Issue #65 (FileSystem): - Remove Value::Array decode path from WriteFile. Require base64 string only. The filter_map over as_u64() silently drops bytes >127 from signed Int8Array serializers, causing silent data corruption. Return a clear error if array is passed. - Extract require_non_empty_path() inline helper; replace the four copy-pasted empty-path guards across ReadFile, WriteFile, ReadDirectory, Stat. - Standardize ordering: validate path before run_time.Environment.Require() in Stat (was inconsistent vs the other three handlers). - Add comment on vscode://schemas-associations/ special-case in ReadFile.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #63, #65
Issue #63 - ActiveOperations map leak
process_cocoon_requestcalledRegisterOperationunconditionally on every RPC but no handler ever calledUnregisterOperation. TheActiveOperationsmap grew without bound — one leakedCancellationTokenper RPC. In long sessions with 14k+ RPCs this is 14k+ live entries.Fix: Remove the unconditional
RegisterOperationcall fromprocess_cocoon_request. Operations that need cancellation must opt-in by callingRegisterOperationfrom within their handler body andUnregisterOperationon both success and error paths. Thecancel_operationpath already removes the entry aftertoken.cancel()— no change needed there.Also removed
ValidateRequestchecks for"::"and"../"in method names. Path traversal is in parameters, not method names; these checks block legitimate callers and provide no security benefit.Issue #65 - FileSystem WriteFile + empty-path guard
WriteFile byte-array decode removed:
Value::Arraypath iterated every element withv.as_u64().map(|n| n as u8)— one enum-match per bytefilter_mapsilently dropped bytes >127 from signedInt8Arrayserializers, causing silent data corruptionEmpty-path guard extracted to
require_non_empty_path():ReadFile,WriteFile,ReadDirectory,StatEnvironment.Require()in all handlers (Stat previously validated after)Comment added on the
vscode://schemas-associations/special-case inReadFile.