Merge pull request #340 from algorandfoundation/strip-comment-fix

fix: prevent base64 in teal from being stripped as a comment

docs/code/classes/types_app_manager.AppManager.md

@@ -524,4 +524,4 @@ The TEAL without comments
 
 #### Defined in
 
-[src/types/app-manager.ts:462](https://github.com/algorandfoundation/algokit-utils-ts/blob/main/src/types/app-manager.ts#L462)
+[src/types/app-manager.ts:463](https://github.com/algorandfoundation/algokit-utils-ts/blob/main/src/types/app-manager.ts#L463)

docs/code/interfaces/types_app_arc56.Arc56Contract.md

@@ -157,7 +157,7 @@ Optional object listing the contract instances across different networks.
 The key is the base64 genesis hash of the network, and the value contains
 information about the deployed contract in the network indicated by the
 key. A key containing the human-readable name of the network MAY be
-included, but the corresponding genesis hash key MUST also be define
+included, but the corresponding genesis hash key MUST also be defined
 
 #### Index signature
 
@@ -257,7 +257,7 @@ ___
 
 • **structs**: `Object`
 
-Named structs use by the application. Each struct field appears in the same order as ABI encoding.
+Named structs used by the application. Each struct field appears in the same order as ABI encoding.
 
 #### Index signature
 
@@ -273,7 +273,7 @@ ___
 
 • `Optional` **templateVariables**: `Object`
 
-A mapping of template variable names as they appear in the teal (not including TMPL_ prefix) to their respective types and values (if applicable)
+A mapping of template variable names as they appear in the TEAL (not including TMPL_ prefix) to their respective types and values (if applicable)
 
 #### Index signature
 

src/app-deploy.spec.ts

@@ -564,42 +564,93 @@ describe('deploy-app', () => {
 })
 
 test('Strip comments remove comments without removing commands', async () => {
-  const tealCode =
-    '//comment\nop arg //comment\nop "arg" //comment\nop "//" //comment\nop "  //comment  " //comment\nop "" //" //comment\nop "" //comment\n//\nop 123\nop 123 // something\nop "" // more comments\nop "//" //op "//"\nop "//"'
-  const tealCodeResult = '\nop arg\nop "arg"\nop "//"\nop "  //comment  "\nop "" //"\nop ""\n\nop 123\nop 123\nop ""\nop "//"\nop "//"'
+  const tealCode = `//comment
+op arg //comment
+op "arg" //comment
+op "//" //comment
+op "  //comment  " //comment
+op "\\" //" //comment
+op ""     //comment
+//
+op 123
+op 123 // something
+op "" // more comments
+op "//" //op "//"
+op "//"
+
+pushbytes base64(//8=)
+pushbytes b64(//8=)
+pushbytes base64(//8=)  // pushbytes base64(//8=)
+pushbytes b64(//8=)     // pushbytes b64(//8=)
+pushbytes "base64(//8=)"  // pushbytes "base64(//8=)"
+pushbytes "b64(//8=)"     // pushbytes "b64(//8=)"
+pushbytes base64 //8=
+pushbytes b64 //8=
+pushbytes base64 //8=  // pushbytes base64 //8=
+pushbytes b64 //8=     // pushbytes b64 //8=
+pushbytes "base64 //8="  // pushbytes "base64 //8="
+pushbytes "b64 //8="     // pushbytes "b64 //8="
+`
+  const tealCodeResult = `
+op arg
+op "arg"
+op "//"
+op "  //comment  "
+op "\\" //"
+op ""
+
+op 123
+op 123
+op ""
+op "//"
+op "//"
+
+pushbytes base64(//8=)
+pushbytes b64(//8=)
+pushbytes base64(//8=)
+pushbytes b64(//8=)
+pushbytes "base64(//8=)"
+pushbytes "b64(//8=)"
+pushbytes base64 //8=
+pushbytes b64 //8=
+pushbytes base64 //8=
+pushbytes b64 //8=
+pushbytes "base64 //8="
+pushbytes "b64 //8="
+`
 
   const result = AppManager.stripTealComments(tealCode)
 
   expect(result).toBe(tealCodeResult)
 })
 
 test('Can substitute template variable with multiple underscores', async () => {
-  const test_teal = `
+  const testTeal = `
   int TMPL_SOME_VALUE
   return
   `
-  const test_params = {
+  const testParams = {
     SOME_VALUE: 123,
   }
-  const substituted = AppManager.replaceTealTemplateParams(test_teal, test_params)
+  const substituted = AppManager.replaceTealTemplateParams(testTeal, testParams)
   expect(substituted).toBe(`
   int 123
   return
   `)
 })
 
 test('Can substitue both bytes and int uint64', async () => {
-  const test_teal = `
+  const testTeal = `
   int TMPL_SOME_VALUE
   pushint TMPL_SOME_VALUE
   bytes TMPL_SOME_VALUE
   pushbytes TMPL_SOME_VALUE
   return
   `
-  const test_params = {
+  const testParams = {
     SOME_VALUE: 123,
   }
-  const substituted = AppManager.replaceTealTemplateParams(test_teal, test_params)
+  const substituted = AppManager.replaceTealTemplateParams(testTeal, testParams)
   expect(substituted).toBe(`
   int 123
   pushint 123
@@ -609,6 +660,58 @@ test('Can substitue both bytes and int uint64', async () => {
   `)
 })
 
+test('Does not substitute template variables in comments or when quoted', async () => {
+  const testTeal = `
+test TMPL_INT // TMPL_INT
+test TMPL_INT
+no change
+test TMPL_STR // TMPL_STR
+TMPL_STR
+TMPL_STR // TMPL_INT
+TMPL_STR // foo //
+TMPL_STR // bar
+test "TMPL_STR" // not replaced
+test "TMPL_STRING" // not replaced
+test TMPL_STRING // not replaced
+test TMPL_STRI // not replaced
+test TMPL_STR TMPL_INT TMPL_INT TMPL_STR // TMPL_STR TMPL_INT TMPL_INT TMPL_STR
+test TMPL_INT TMPL_STR TMPL_STRING "TMPL_INT TMPL_STR TMPL_STRING" //TMPL_INT TMPL_STR TMPL_STRING
+test TMPL_INT TMPL_INT TMPL_STRING TMPL_STRING TMPL_STRING TMPL_INT TMPL_STRING //keep
+TMPL_STR TMPL_STR TMPL_STR
+TMPL_STRING
+test NOTTMPL_STR // not replaced
+NOTTMPL_STR // not replaced
+TMPL_STR // replaced
+  `
+  const testParams = {
+    INT: 123,
+    STR: 'ABC',
+  }
+  const substituted = AppManager.replaceTealTemplateParams(testTeal, testParams)
+  expect(substituted).toBe(`
+test 123 // TMPL_INT
+test 123
+no change
+test 0x414243 // TMPL_STR
+0x414243
+0x414243 // TMPL_INT
+0x414243 // foo //
+0x414243 // bar
+test "TMPL_STR" // not replaced
+test "TMPL_STRING" // not replaced
+test TMPL_STRING // not replaced
+test TMPL_STRI // not replaced
+test 0x414243 123 123 0x414243 // TMPL_STR TMPL_INT TMPL_INT TMPL_STR
+test 123 0x414243 TMPL_STRING "TMPL_INT TMPL_STR TMPL_STRING" //TMPL_INT TMPL_STR TMPL_STRING
+test 123 123 TMPL_STRING TMPL_STRING TMPL_STRING 123 TMPL_STRING //keep
+0x414243 0x414243 0x414243
+TMPL_STRING
+test NOTTMPL_STR // not replaced
+NOTTMPL_STR // not replaced
+0x414243 // replaced
+  `)
+})
+
 function getMetadata(overrides?: Partial<AppDeployMetadata>): AppDeployMetadata {
   return {
     name: 'test',

src/app-deploy.ts

@@ -330,15 +330,5 @@ export async function performTemplateSubstitutionAndCompile(
  * @returns The TEAL without comments
  */
 export function stripTealComments(tealCode: string) {
-  // find // outside quotes, i.e. won't pick up "//not a comment"
-  const regex = /\/\/(?=([^"\\]*(\\.|"([^"\\]*\\.)*[^"\\]*"))*[^"]*$)/
-
-  tealCode = tealCode
-    .split('\n')
-    .map((tealCodeLine) => {
-      return tealCodeLine.split(regex)[0].trim()
-    })
-    .join('\n')
-
-  return tealCode
+  return AppManager.stripTealComments(tealCode)
 }

src/types/app-arc56.ts

@@ -240,15 +240,15 @@ export interface Arc56Contract {
    * The key is the base64 genesis hash of the network, and the value contains
    * information about the deployed contract in the network indicated by the
    * key. A key containing the human-readable name of the network MAY be
-   * included, but the corresponding genesis hash key MUST also be define
+   * included, but the corresponding genesis hash key MUST also be defined
    */
   networks?: {
     [network: string]: {
       /** The app ID of the deployed contract in this network */
       appID: number
     }
   }
-  /** Named structs use by the application. Each struct field appears in the same order as ABI encoding. */
+  /** Named structs used by the application. Each struct field appears in the same order as ABI encoding. */
   structs: { [structName: StructName]: StructField[] }
   /** All of the methods that the contract implements */
   methods: Method[]
@@ -319,12 +319,12 @@ export interface Arc56Contract {
   }
   /** ARC-28 events that MAY be emitted by this contract */
   events?: Array<Event>
-  /** A mapping of template variable names as they appear in the teal (not including TMPL_ prefix) to their respective types and values (if applicable) */
+  /** A mapping of template variable names as they appear in the TEAL (not including TMPL_ prefix) to their respective types and values (if applicable) */
   templateVariables?: {
     [name: string]: {
       /** The type of the template variable */
       type: ABIType | AVMType | StructName
-      /** If given, the the base64 encoded value used for the given app/program */
+      /** If given, the base64 encoded value used for the given app/program */
       value?: string
     }
   }

src/types/app-manager.ts

@@ -401,7 +401,7 @@ export class AppManager {
           `Deploy-time updatability control requested for app deployment, but ${UPDATABLE_TEMPLATE_NAME} not present in TEAL code`,
         )
       }
-      tealTemplateCode = tealTemplateCode.replace(new RegExp(UPDATABLE_TEMPLATE_NAME, 'g'), (params.updatable ? 1 : 0).toString())
+      tealTemplateCode = replaceTemplateVariable(tealTemplateCode, UPDATABLE_TEMPLATE_NAME, (params.updatable ? 1 : 0).toString())
     }
 
     if (params.deletable !== undefined) {
@@ -410,7 +410,7 @@ export class AppManager {
           `Deploy-time deletability control requested for app deployment, but ${DELETABLE_TEMPLATE_NAME} not present in TEAL code`,
         )
       }
-      tealTemplateCode = tealTemplateCode.replace(new RegExp(DELETABLE_TEMPLATE_NAME, 'g'), (params.deletable ? 1 : 0).toString())
+      tealTemplateCode = replaceTemplateVariable(tealTemplateCode, DELETABLE_TEMPLATE_NAME, (params.deletable ? 1 : 0).toString())
     }
 
     return tealTemplateCode
@@ -439,8 +439,9 @@ export class AppManager {
           // We could probably return here since mixing pushint and pushbytes is likely not going to happen, but might as well do both
         }
 
-        tealTemplateCode = tealTemplateCode.replace(
-          new RegExp(token, 'g'),
+        tealTemplateCode = replaceTemplateVariable(
+          tealTemplateCode,
+          token,
           typeof value === 'string'
             ? `0x${Buffer.from(value, 'utf-8').toString('hex')}`
             : ArrayBuffer.isView(value)
@@ -460,16 +461,116 @@ export class AppManager {
    * @returns The TEAL without comments
    */
   static stripTealComments(tealCode: string) {
-    // find // outside quotes, i.e. won't pick up "//not a comment"
-    const regex = /\/\/(?=([^"\\]*(\\.|"([^"\\]*\\.)*[^"\\]*"))*[^"]*$)/
+    const stripCommentFromLine = (line: string) => {
+      const commentIndex = findUnquotedString(line, '//')
+      if (commentIndex === undefined) {
+        return line
+      }
+      return line.slice(0, commentIndex).trimEnd()
+    }
 
-    tealCode = tealCode
+    return tealCode
       .split('\n')
-      .map((tealCodeLine) => {
-        return tealCodeLine.split(regex)[0].trim()
-      })
+      .map((line) => stripCommentFromLine(line))
       .join('\n')
+  }
+}
 
-    return tealCode
+/**
+ * Find the first string within a line of TEAL. Only matches outside of quotes and base64 are returned.
+ * Returns undefined if not found
+ */
+const findUnquotedString = (line: string, token: string, startIndex: number = 0, _endIndex?: number): number | undefined => {
+  const endIndex = _endIndex === undefined ? line.length : _endIndex
+  let index = startIndex
+  let inQuotes = false
+  let inBase64 = false
+
+  while (index < endIndex) {
+    const currentChar = line[index]
+    if ((currentChar === ' ' || currentChar === '(') && !inQuotes && lastTokenBase64(line, index)) {
+      // enter base64
+      inBase64 = true
+    } else if ((currentChar === ' ' || currentChar === ')') && !inQuotes && inBase64) {
+      // exit base64
+      inBase64 = false
+    } else if (currentChar === '\\' && inQuotes) {
+      // escaped char, skip next character
+      index += 1
+    } else if (currentChar === '"') {
+      // quote boundary
+      inQuotes = !inQuotes
+    } else if (!inQuotes && !inBase64 && line.startsWith(token, index)) {
+      // can test for match
+      return index
+    }
+    index += 1
+  }
+  return undefined
+}
+
+const lastTokenBase64 = (line: string, index: number): boolean => {
+  try {
+    const tokens = line.slice(0, index).split(/\s+/)
+    const last = tokens[tokens.length - 1]
+    return ['base64', 'b64'].includes(last)
+  } catch {
+    return false
   }
 }
+
+function replaceTemplateVariable(program: string, token: string, replacement: string): string {
+  const result: string[] = []
+  const tokenIndexOffset = replacement.length - token.length
+
+  const programLines = program.split('\n')
+
+  for (const line of programLines) {
+    const _commentIndex = findUnquotedString(line, '//')
+    const commentIndex = _commentIndex === undefined ? line.length : _commentIndex
+    let code = line.substring(0, commentIndex)
+    const comment = line.substring(commentIndex)
+    let trailingIndex = 0
+
+    // eslint-disable-next-line no-constant-condition
+    while (true) {
+      const tokenIndex = findTemplateToken(code, token, trailingIndex)
+      if (tokenIndex === undefined) {
+        break
+      }
+      trailingIndex = tokenIndex + token.length
+      const prefix = code.substring(0, tokenIndex)
+      const suffix = code.substring(trailingIndex)
+      code = `${prefix}${replacement}${suffix}`
+      trailingIndex += tokenIndexOffset
+    }
+    result.push(code + comment)
+  }
+
+  return result.join('\n')
+}
+
+const findTemplateToken = (line: string, token: string, startIndex: number = 0, _endIndex?: number): number | undefined => {
+  const endIndex = _endIndex === undefined ? line.length : _endIndex
+
+  let index = startIndex
+  while (index < endIndex) {
+    const tokenIndex = findUnquotedString(line, token, index, endIndex)
+    if (tokenIndex === undefined) {
+      break
+    }
+    const trailingIndex = tokenIndex + token.length
+    if (
+      (tokenIndex === 0 || !isValidTokenCharacter(line[tokenIndex - 1])) &&
+      (trailingIndex >= line.length || !isValidTokenCharacter(line[trailingIndex]))
+    ) {
+      return tokenIndex
+    }
+    index = trailingIndex
+  }
+  return undefined
+}
+
+function isValidTokenCharacter(char: string): boolean {
+  return char.length === 1 && (/\w/.test(char) || char === '_')
+}