fix command escaping (#302)

actions · Jan 19, 2020 · 8b03001 · 8b03001
1 parent ab5bd9d
commit 8b03001
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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')
+  return (s || '')
+    .replace(/%/g, '%25')
+    .replace(/\r/g, '%0D')
+    .replace(/\n/g, '%0A')
 }
 
-function escape(s: string): string {
-  return s
+function escapeProperty(s: string): string {
+  return (s || '')
+    .replace(/%/g, '%25')
     .replace(/\r/g, '%0D')
     .replace(/\n/g, '%0A')
-    .replace(/]/g, '%5D')
-    .replace(/;/g, '%3B')
+    .replace(/:/g, '%3A')
+    .replace(/,/g, '%2C')
 }