From 8b0300129f08728419263b016de8630f1d426d5f Mon Sep 17 00:00:00 2001
From: eric sciple <ericsciple@users.noreply.github.com>
Date: Sat, 18 Jan 2020 23:52:44 -0500
Subject: [PATCH] fix command escaping (#302)

---
 packages/core/__tests__/command.test.ts | 45 +++++++++++++++++++++++++
 packages/core/__tests__/core.test.ts    |  6 ++--
 packages/core/src/command.ts            | 36 +++++++++-----------
 3 files changed, 64 insertions(+), 23 deletions(-)

diff --git a/packages/core/__tests__/command.test.ts b/packages/core/__tests__/command.test.ts
index 0fa99e97..182e145c 100644
--- a/packages/core/__tests__/command.test.ts
+++ b/packages/core/__tests__/command.test.ts
@@ -27,6 +27,51 @@ describe('@actions/core/src/command', () => {
     assertWriteCalls([`::some-command::${os.EOL}`])
   })
 
+  it('command escapes message', () => {
+    // Verify replaces each instance, not just first instance
+    command.issueCommand(
+      'some-command',
+      {},
+      'percent % percent % cr \r cr \r lf \n lf \n'
+    )
+    assertWriteCalls([
+      `::some-command::percent %25 percent %25 cr %0D cr %0D lf %0A lf %0A${os.EOL}`
+    ])
+
+    // Verify literal escape sequences
+    process.stdout.write = jest.fn()
+    command.issueCommand('some-command', {}, '%25 %25 %0D %0D %0A %0A')
+    assertWriteCalls([
+      `::some-command::%2525 %2525 %250D %250D %250A %250A${os.EOL}`
+    ])
+  })
+
+  it('command escapes property', () => {
+    // Verify replaces each instance, not just first instance
+    command.issueCommand(
+      'some-command',
+      {
+        name:
+          'percent % percent % cr \r cr \r lf \n lf \n colon : colon : comma , comma ,'
+      },
+      ''
+    )
+    assertWriteCalls([
+      `::some-command name=percent %25 percent %25 cr %0D cr %0D lf %0A lf %0A colon %3A colon %3A comma %2C comma %2C::${os.EOL}`
+    ])
+
+    // Verify literal escape sequences
+    process.stdout.write = jest.fn()
+    command.issueCommand(
+      'some-command',
+      {},
+      '%25 %25 %0D %0D %0A %0A %3A %3A %2C %2C'
+    )
+    assertWriteCalls([
+      `::some-command::%2525 %2525 %250D %250D %250A %250A %253A %253A %252C %252C${os.EOL}`
+    ])
+  })
+
   it('command with message', () => {
     command.issueCommand('some-command', {}, 'some message')
     assertWriteCalls([`::some-command::some message${os.EOL}`])
diff --git a/packages/core/__tests__/core.test.ts b/packages/core/__tests__/core.test.ts
index 2c7e0b54..90235428 100644
--- a/packages/core/__tests__/core.test.ts
+++ b/packages/core/__tests__/core.test.ts
@@ -41,10 +41,10 @@ describe('@actions/core', () => {
   })
 
   it('exportVariable escapes variable names', () => {
-    core.exportVariable('special char var \r\n];', 'special val')
-    expect(process.env['special char var \r\n];']).toBe('special val')
+    core.exportVariable('special char var \r\n,:', 'special val')
+    expect(process.env['special char var \r\n,:']).toBe('special val')
     assertWriteCalls([
-      `::set-env name=special char var %0D%0A%5D%3B::special val${os.EOL}`
+      `::set-env name=special char var %0D%0A%2C%3A::special val${os.EOL}`
     ])
   })
 
diff --git a/packages/core/src/command.ts b/packages/core/src/command.ts
index 29185dfc..9bdfd9c4 100644
--- a/packages/core/src/command.ts
+++ b/packages/core/src/command.ts
@@ -10,11 +10,11 @@ interface CommandProperties {
  * Commands
  *
  * Command Format:
- *   ##[name key=value;key=value]message
+ *   ::name key=value,key=value::message
  *
  * Examples:
- *   ##[warning]This is the user warning message
- *   ##[set-secret name=mypassword]definitelyNotAPassword!
+ *   ::warning::This is the message
+ *   ::set-env name=MY_VAR::some value
  */
 export function issueCommand(
   command: string,
@@ -62,33 +62,29 @@ class Command {
               cmdStr += ','
             }
 
-            // safely append the val - avoid blowing up when attempting to
-            // call .replace() if message is not a string for some reason
-            cmdStr += `${key}=${escape(`${val || ''}`)}`
+            cmdStr += `${key}=${escapeProperty(val)}`
           }
         }
       }
     }
 
-    cmdStr += CMD_STRING
-
-    // safely append the message - avoid blowing up when attempting to
-    // call .replace() if message is not a string for some reason
-    const message = `${this.message || ''}`
-    cmdStr += escapeData(message)
-
+    cmdStr += `${CMD_STRING}${escapeData(this.message)}`
     return cmdStr
   }
 }
 
 function escapeData(s: string): string {
-  return s.replace(/\r/g, '%0D').replace(/\n/g, '%0A')
-}
-
-function escape(s: string): string {
-  return s
+  return (s || '')
+    .replace(/%/g, '%25')
     .replace(/\r/g, '%0D')
     .replace(/\n/g, '%0A')
-    .replace(/]/g, '%5D')
-    .replace(/;/g, '%3B')
+}
+
+function escapeProperty(s: string): string {
+  return (s || '')
+    .replace(/%/g, '%25')
+    .replace(/\r/g, '%0D')
+    .replace(/\n/g, '%0A')
+    .replace(/:/g, '%3A')
+    .replace(/,/g, '%2C')
 }