Skip to content

Normalize incoming Home RPC path arguments to the remote host OS #5409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
huyhoang171106 wants to merge 1 commit into
base: develop
Choose a base branch
from
+53 −0
Open

Normalize incoming Home RPC path arguments to the remote host OS #5409

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions platformio/home/rpc/handlers/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,58 @@
# limitations under the License.


import inspect
import os


class BaseRPCHandler:
PATH_ARGUMENT_MARKERS = (
"path",
"paths",
"dir",
"dirs",
"directory",
"directories",
"file",
"files",
"folder",
"folders",
)

factory = None

def __getattribute__(self, name):
attr = super().__getattribute__(name)
if name.startswith("_") or not callable(attr):
return attr

def wrapped(*args, **kwargs):
args, kwargs = self._normalize_path_arguments(attr, args, kwargs)
return attr(*args, **kwargs)

return wrapped
Comment on lines 34 to +45
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__getattribute__ wraps any callable attribute, which includes factory (the JSONRPC server factory instance is callable because it implements __call__). This will cause self.factory to return the wrapper function instead of the factory object, breaking usages like self.factory.notify_clients(...) and self.factory.manager... across RPC handlers. Fix by only wrapping callables that are methods defined on the handler class (e.g., use inspect.getattr_static(type(self), name) to detect function/staticmethod/classmethod descriptors) and leave instance attributes like factory untouched.

Copilot uses AI. Check for mistakes.
Comment on lines +41 to +45
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wrapper returned from __getattribute__ does not preserve the original callable’s metadata/signature (it will look like (*args, **kwargs) to introspection). JSON-RPC dispatchers commonly inspect method signatures to validate/bind params; this can break named-parameter calls and makes debugging harder. Use functools.wraps(attr) (and/or set __signature__) so introspection continues to reflect the original method.

Copilot uses AI. Check for mistakes.

def _normalize_path_arguments(self, method, args, kwargs):
bound_args = inspect.signature(method).bind_partial(*args, **kwargs)
for param_name, value in bound_args.arguments.items():
if not self._is_path_argument(param_name):
continue
bound_args.arguments[param_name] = self._normalize_path_value(value)
return bound_args.args, bound_args.kwargs

def _is_path_argument(self, param_name):
parts = param_name.lower().split("_")
return any(marker in parts for marker in self.PATH_ARGUMENT_MARKERS)

def _normalize_path_value(self, value):
if not value:
return value
if isinstance(value, str):
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.path.normpath normalizes using the host OS separator, but on POSIX it will not convert Windows-style backslashes (\) into / because os.path.altsep is None. If incoming RPC paths can contain mixed separators (common when UI runs on a different OS via SSH tunneling), this won’t actually normalize to the remote host OS. Consider normalizing separators first (e.g., replace both / and \ with os.sep when os.altsep is not set) before calling normpath.

Suggested change
if isinstance(value, str):
if isinstance(value, str):
sep = os.sep
altsep = os.path.altsep
# Normalize alternative separators to the host OS separator before normpath.
if altsep:
value = value.replace(altsep, sep)
else:
# On POSIX, treat backslashes from Windows-style paths as separators.
value = value.replace("\\", sep)

Copilot uses AI. Check for mistakes.
return os.path.normpath(value)
Comment on lines +59 to +63
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_normalize_path_value handles strings and some iterables, but dicts are left unchanged and fall through to return value. RPC params often contain nested objects (e.g., project configuration dicts with a location path), so those nested paths won’t be normalized. Consider adding dict recursion (normalize values for keys matching the path markers / known path keys like location).

Copilot uses AI. Check for mistakes.
if isinstance(value, list):
return [self._normalize_path_value(item) for item in value]
if isinstance(value, tuple):
return tuple(self._normalize_path_value(item) for item in value)
if isinstance(value, set):
return {self._normalize_path_value(item) for item in value}
return value
Loading