Page MenuHomePhabricator

Allow PhutilCommandString to escape as Powershell
ClosedPublic

Authored by hach-que on Aug 13 2014, 1:37 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 30, 4:00 PM
Unknown Object (File)
Fri, Apr 26, 2:47 PM
Unknown Object (File)
Mon, Apr 8, 1:35 PM
Unknown Object (File)
Apr 2 2024, 5:05 AM
Unknown Object (File)
Mar 29 2024, 10:57 PM
Unknown Object (File)
Mar 29 2024, 10:53 AM
Unknown Object (File)
Mar 5 2024, 3:04 AM
Unknown Object (File)
Feb 6 2024, 9:05 AM
Subscribers

Details

Summary

This allows PhutilCommandString to escape as Powershell.

Test Plan

Unit tests

Diff Detail

Repository
rPHU libphutil
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

hach-que retitled this revision from to Allow PhutilCommandString to escape as Powershell.
hach-que updated this object.
hach-que edited the test plan for this revision. (Show Details)
hach-que added a reviewer: epriestley.
hach-que added a subscriber: epriestley.
src/xsprintf/csprintf.php
66

Putting:

if ($is_powershell) {
  throw new Exception('blah');
}

does get hit and causes __toString() can not throw exception in the daemon log.

164

But the command that's run is never this, and always appears to use UNIX escaping.

hach-que edited edge metadata.

Now working

epriestley edited edge metadata.
epriestley added inline comments.
src/xsprintf/PhutilCommandString.php
33–36

Let's make this setEscapingMode(PhutilCommandString::MODE_POWERSHELL) or similar.

43

...then pass 'mode'

src/xsprintf/__tests__/PhutilCsprintfTestCase.php
27

Then eventually this test would become setEscapingMode(PhutilCommandString::MODE_LINUX) and the test would work under windows, in a future far away.

src/xsprintf/csprintf.php
111–112

Let's move this to PhutilCommandString::escapeArgument($argument, $mode = PhutilCommandString::MODE_DEFAULT) instead of defining a new escapepowershell. That should reduce code duplication too, and make real modal operation easy to implement later.

149

In single quotes, I think this means literally "backslash a".

150

Same issue here, this is "backslash b"

This revision now requires changes to proceed.Aug 13 2014, 2:07 AM
hach-que edited edge metadata.

Changes based on feedback

hach-que edited edge metadata.

Change \a and \b to use chr()

epriestley edited edge metadata.
This revision is now accepted and ready to land.Aug 13 2014, 2:34 AM
hach-que updated this revision to Diff 24672.

Closed by commit rPHU49f08a756a54 (authored by @hach-que).