Skip to content
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.

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

Fix Issue 21897 Semicolon inside quotes in path variable causes range violation in pipedmd #133

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MetaLang
Copy link
Member

@MetaLang MetaLang commented May 6, 2021

@MetaLang
Copy link
Member Author

MetaLang commented May 6, 2021

If anyone can point out how I can add some tests for this case, please do so - it isn't immediately obvious.

@MetaLang MetaLang marked this pull request as ready for review May 7, 2021 02:01
@rainers
Copy link
Member

rainers commented May 7, 2021

Not sure if it is worth the trouble, but ';' is a valid path character and is searched if it is specified in PATH with quotes. To mimick this behavior PATH should not be split simply by ; as done above the patch. This would also avoid the trouble with mismatched quotes in the report.

Skipping empty paths is fine (I guess this avoids the RangeError), but removing just a pair of quotes should be slightly more correct.

If anyone can point out how I can add some tests for this case, please do so - it isn't immediately obvious.

Sorry, there are no tests for pipedmd ATM.

@MetaLang
Copy link
Member Author

MetaLang commented May 11, 2021

To mimick this behavior PATH should not be split simply by ; as done above the patch. This would also avoid the trouble with mismatched quotes in the report.

What would you suggest instead of splitting the path by ;? Splitting by ";?

Skipping empty paths is fine (I guess this avoids the RangeError), but removing just a pair of quotes should be slightly more correct.

I'm not clear on what you mean by this. Can you give an example?

@rainers
Copy link
Member

rainers commented May 11, 2021

What would you suggest instead of splitting the path by ;? Splitting by ";?

I think that the splitting by ';' must not occur between matching quotes. To avoid that PATH has to be parsed manually.

I'm not clear on what you mean by this. Can you give an example?

I meant that quotes should only be removed as a pair at the beginning and the end of a path, e.g. for "c:\a\b", but not for "c:\a\b. Given the rule above that matching quotes are searched for anyway, it only makes a difference for the last quote without a match, and this actually seems to be removed by cmd. So strip is fine indeed. Actually, all quotes should be removed from the resulting path p.

rainers added a commit to rainers/visuald that referenced this pull request May 23, 2021
@rainers
Copy link
Member

rainers commented May 23, 2021

I have implemented proper path splitting here now: rainers@c62c2cf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants