Skip to content

fix(pedm): fix flawed expand_environment function #1366

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

Jump to bottom
Merged
CBenoit merged 1 commit into from
May 28, 2025
Merged

fix(pedm): fix flawed expand_environment function #1366

CBenoit merged 1 commit into from
May 28, 2025

Conversation

@thenextman
Copy link
Member

@thenextman thenextman commented May 28, 2025

expand_environment is flawed; because it splits the input on "%" there can be improper boundary detection. For example, given input with back-to-back variables like "%HOMEDIR%%HOMEPATH%".

Instead, we walk the string one character at a time and properly tokenize the variables. This still account for the malformed example in Kyle's original comment ("For strings such as "%MyVar%MyVar%", only the first occurrence should be replaced").

@thenextman thenextman requested a review from a team May 28, 2025 19:21
@github-actions
Copy link

github-actions bot commented May 28, 2025

Let maintainers know that an action is required on their side

  • Add the label release-required Please cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module) when you request a maintainer to cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module)

  • Add the label release-blocker Follow-up is required before cutting a new release if a follow-up is required before cutting a new release

  • Add the label publish-required Please publish libraries (`Devolutions.Gateway.Utils`, OpenAPI clients, etc) when you request a maintainer to publish libraries (Devolutions.Gateway.Utils, OpenAPI clients, etc.)

  • Add the label publish-blocker Follow-up is required before publishing libraries if a follow-up is required before publishing libraries

@thenextman thenextman force-pushed the pedm-fix-expand-environment branch from 26ae8c9 to 95e0fc4 Compare May 28, 2025 19:24
CBenoit
CBenoit approved these changes May 28, 2025
View reviewed changes
Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@CBenoit CBenoit merged commit 476b885 into master May 28, 2025
39 checks passed
@CBenoit CBenoit deleted the pedm-fix-expand-environment branch May 28, 2025 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CBenoit CBenoit CBenoit approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@thenextman @CBenoit