fix: enhance process exclusion logic and improve configuration documentation (PR #84) #85
Conversation
Revised NukaColaM's code (#46).
Fixes for PR #84 from https://github.com/PoneyClairDeLune - Fix case-insensitive matching in socks_local_router exclusion logic * Update exclusion checks to follow same path/name selection logic as pattern matching * Remove redundant to_upper() calls since network_process fields are already uppercase * Ensure consistent behavior between exclusion and association logic - Add comprehensive documentation for match_app_name function * Document two-stage matching process (exclusion then pattern matching) * Clarify intelligent field selection based on path separators * Add detailed parameter descriptions with examples - Update configuration documentation and examples * Add missing excludedList parameter documentation in ProxiFyreSettings constructor * Include "excludes" array in JSON configuration example with various exclusion patterns * Demonstrate support for process names, full paths, and partial path matching - Enhance code consistency across C++ and C# components * Align exclusion logic implementation with documented behavior * Improve documentation completeness for better maintainability These changes ensure robust process filtering with consistent case-insensitive matching and comprehensive documentation for configuration and usage.
There was a problem hiding this comment.
Summary of Changes
Hello @wiresock, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly improves the flexibility and control of the proxy service by implementing a dynamic process exclusion feature. Users can now define a list of applications or paths that should bypass the proxy, enhancing the service's adaptability to various network environments and user requirements. The changes span across the configuration, core network logic, and the interop layers to seamlessly integrate this new capability.
Highlights
- Enhanced Process Exclusion: Introduced a new mechanism to dynamically exclude processes from being proxied based on a configurable list. This allows for more granular control over which applications' traffic is routed through the SOCKS5 proxy.
- Configuration Updates: The application's configuration now supports an 'excludes' section, allowing users to specify process names or paths to be excluded from proxying. Example configuration has been updated to reflect this.
- Core Logic Refinement: The underlying network library (
netlib) has been updated to incorporate the new exclusion logic within itsmatch_app_namefunction, ensuring that excluded processes are identified and bypassed before standard proxy matching occurs. - API Exposure: New methods have been added to the
Socksifierandsocksify_unmanagedlayers to expose the process exclusion functionality, making it accessible from the managed C# application.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request enhances the process exclusion logic by allowing dynamic exclusion of processes from being proxied, and also improves the configuration documentation. The changes are well-structured, introducing a new excludes list in the configuration and propagating it down to the native netlib library. My review includes suggestions to improve exception handling by logging error details, which will be valuable for debugging, and fixing a small typo in the documentation.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Implement full result caching for match_app_name function * Cache complete matching results (not just exclusions) using process ID + app pattern key * Add composite cache key with custom hash function for optimal performance * Implement time-based expiration (30 seconds TTL) and size-limited cache (2000 entries) * Add automatic cleanup for expired and oldest entries to prevent memory bloat - Enhance cache management and invalidation * Clear cache when exclusion list or proxy mappings change via clear_match_cache() * Update associate_process_name_to_proxy and exclude_process_name to invalidate cache * Add comprehensive cache cleanup strategy with timestamp-based sorting - Optimize packet processing hot path performance * Eliminate repeated string operations for same process/app combinations * Convert O(n) linear searches to O(1) hash map lookups for cached results * Maintain thread safety with mutex protection for concurrent access - Update documentation to reflect caching behavior * Document comprehensive result caching in match_app_name function * Add detailed comments for cache data structures and management * Explain performance benefits and cache invalidation strategy This optimization significantly improves performance for high-traffic scenarios where the same processes generate many packets, eliminating redundant exclusion checks and pattern matching operations through intelligent caching.
- Remove mutex acquisition from cleanup_match_cache() function - Function now assumes caller holds match_cache_mutex_ (which is correct) - Eliminates unsafe lock unlock/relock pattern in match_app_name() - Simplifies code flow with single lock ownership per operation - Called only from match_app_name() where lock is already held This change improves thread safety by removing complex mutex manipulation and clarifies lock ownership responsibilities. The cache cleanup is now a simple utility function that operates under the caller's lock protection.
- Make excludedList parameter optional with default null value in ProxiFyreSettings constructor - Add NullValueHandling.Ignore to JsonProperty attribute for ExcludedList - Ensure ExcludedList is always initialized as empty list when field is missing from JSON - Remove duplicate class summary comment for ProxiFyreSettings This change prevents deserialization errors when app-config.json doesn't contain the "excludes" field, making the exclusion configuration truly optional while maintaining backward compatibility with existing configurations.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the process exclusion functionality and improves configuration documentation. It adds the ability to exclude specific processes from being proxied and implements a caching mechanism to optimize performance for repeated process matching operations.
- Adds new process exclusion APIs across C++ unmanaged and C# managed layers
- Implements intelligent caching for process matching with TTL-based expiration
- Enhances configuration with support for process exclusion lists
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| socksify/socksify_unmanaged.h | Adds declaration for exclude_process_name method |
| socksify/socksify_unmanaged.cpp | Implements exclude_process_name wrapper method with documentation |
| socksify/Socksifier.h | Adds managed wrapper declaration for ExcludeProcessName |
| socksify/Socksifier.cpp | Implements managed ExcludeProcessName method |
| netlib/src/proxy/socks_local_router.h | Adds exclusion list, caching infrastructure, and enhanced process matching logic |
| ProxiFyre/Program.cs | Integrates exclusion list processing and updates configuration documentation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ProxiFyre/Program.cs
Outdated
| /// <summary> | ||
| /// Represents the root configuration settings for ProxiFyre. | ||
| /// </summary> |
There was a problem hiding this comment.
There are duplicate XML documentation comments for the ProxiFyreSettings class. Remove the duplicate summary tags on lines 158-160.
| /// <summary> | |
| /// Represents the root configuration settings for ProxiFyre. | |
| /// </summary> | |
| // Check exclusion list | ||
| for (const auto& excluded_entry : excluded_list_) { | ||
| if ((excluded_entry.find(L'\\') != std::wstring::npos || excluded_entry.find(L'/') != std::wstring::npos) | ||
| ? (process->path_name.find(excluded_entry) != std::wstring::npos) | ||
| : (process->name.find(excluded_entry) != std::wstring::npos) | ||
| ) { | ||
| matches = false; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
The exclusion list is checked on every call without caching. Since excluded_list_ is modified infrequently but checked frequently, consider caching the exclusion check results or using a more efficient data structure like std::unordered_set for O(1) lookups instead of linear search.
| for (const auto& excluded_entry : excluded_list_) { | ||
| if ((excluded_entry.find(L'\\') != std::wstring::npos || excluded_entry.find(L'/') != std::wstring::npos) | ||
| ? (process->path_name.find(excluded_entry) != std::wstring::npos) | ||
| : (process->name.find(excluded_entry) != std::wstring::npos) |
There was a problem hiding this comment.
Case-insensitive comparison is not being performed for exclusion checks, but the exclusion entries are stored with to_upper() in exclude_process_name(). The process->path_name and process->name should be converted to uppercase before comparison, similar to how pattern matching is done on lines 942-944.
| for (const auto& excluded_entry : excluded_list_) { | |
| if ((excluded_entry.find(L'\\') != std::wstring::npos || excluded_entry.find(L'/') != std::wstring::npos) | |
| ? (process->path_name.find(excluded_entry) != std::wstring::npos) | |
| : (process->name.find(excluded_entry) != std::wstring::npos) | |
| // Convert process path and name to uppercase for case-insensitive comparison | |
| const auto process_path_upper = to_upper(process->path_name); | |
| const auto process_name_upper = to_upper(process->name); | |
| for (const auto& excluded_entry : excluded_list_) { | |
| if ((excluded_entry.find(L'\\') != std::wstring::npos || excluded_entry.find(L'/') != std::wstring::npos) | |
| ? (process_path_upper.find(excluded_entry) != std::wstring::npos) | |
| : (process_name_upper.find(excluded_entry) != std::wstring::npos) |
No description provided.