From f81695570491ede77975de2c26a07612a2d62c28 Mon Sep 17 00:00:00 2001
From: Joel Hamilton <joelhamilton5@gmail.com>
Date: Sun, 18 Feb 2024 00:30:16 -0500
Subject: [PATCH] fix: in conditionals, don't render anything after an else
 branch (#671)

* fix: only render the first 'else' template in the case of multiples

* fix: empty else block and cases with when conditions

* fix: don't render elsif after else

* chore: Update src/tags/unless.ts

* chore: Update src/tags/if.ts

* chore: Update src/tags/case.ts

---------

Co-authored-by: Jun Yang <harttleharttle@gmail.com>
---
 src/tags/case.ts                     | 16 +++++++++++++--
 src/tags/if.ts                       | 28 +++++++++++++++++++-------
 src/tags/unless.ts                   | 30 ++++++++++++++++++++--------
 test/e2e/issues.spec.ts              | 16 +++++++++++++++
 test/integration/tags/case.spec.ts   | 28 ++++++++++++++++++++++++++
 test/integration/tags/if.spec.ts     |  8 ++++++++
 test/integration/tags/unless.spec.ts | 23 +++++++++++++++++++++
 7 files changed, 132 insertions(+), 17 deletions(-)

diff --git a/src/tags/case.ts b/src/tags/case.ts
index 23564d3a8..de0d2d651 100644
--- a/src/tags/case.ts
+++ b/src/tags/case.ts
@@ -10,8 +10,13 @@ export default class extends Tag {
     this.elseTemplates = []
 
     let p: Template[] = []
+    let elseCount = 0
     const stream: ParseStream = this.liquid.parser.parseStream(remainTokens)
       .on('tag:when', (token: TagToken) => {
+        if (elseCount > 0) {
+          return
+        }
+
         p = []
 
         const values: ValueToken[] = []
@@ -29,9 +34,16 @@ export default class extends Tag {
           templates: p
         })
       })
-      .on('tag:else', () => (p = this.elseTemplates))
+      .on('tag:else', () => {
+        elseCount++
+        p = this.elseTemplates
+      })
       .on('tag:endcase', () => stream.stop())
-      .on('template', (tpl: Template) => p.push(tpl))
+      .on('template', (tpl: Template) => {
+        if (p !== this.elseTemplates || elseCount === 1) {
+          p.push(tpl)
+        }
+      })
       .on('end', () => {
         throw new Error(`tag ${tagToken.getText()} not closed`)
       })
diff --git a/src/tags/if.ts b/src/tags/if.ts
index 8e6651793..f563c8060 100644
--- a/src/tags/if.ts
+++ b/src/tags/if.ts
@@ -6,19 +6,33 @@ export default class extends Tag {
 
   constructor (tagToken: TagToken, remainTokens: TopLevelToken[], liquid: Liquid) {
     super(tagToken, remainTokens, liquid)
-    let p
+    let p: Template[] = []
+    let elseCount = 0
     liquid.parser.parseStream(remainTokens)
       .on('start', () => this.branches.push({
         value: new Value(tagToken.args, this.liquid),
         templates: (p = [])
       }))
-      .on('tag:elsif', (token: TagToken) => this.branches.push({
-        value: new Value(token.args, this.liquid),
-        templates: (p = [])
-      }))
-      .on('tag:else', () => (p = this.elseTemplates))
+      .on('tag:elsif', (token: TagToken) => {
+        if (elseCount > 0) {
+          p = []
+          return
+        }
+        this.branches.push({
+          value: new Value(token.args, this.liquid),
+          templates: (p = [])
+        })
+      })
+      .on('tag:else', () => {
+        elseCount++
+        p = this.elseTemplates
+      })
       .on('tag:endif', function () { this.stop() })
-      .on('template', (tpl: Template) => p.push(tpl))
+      .on('template', (tpl: Template) => {
+        if (p !== this.elseTemplates || elseCount === 1) {
+          p.push(tpl)
+        }
+      })
       .on('end', () => { throw new Error(`tag ${tagToken.getText()} not closed`) })
       .start()
   }
diff --git a/src/tags/unless.ts b/src/tags/unless.ts
index 390d7931b..56216aa5c 100644
--- a/src/tags/unless.ts
+++ b/src/tags/unless.ts
@@ -5,21 +5,35 @@ export default class extends Tag {
   elseTemplates: Template[] = []
   constructor (tagToken: TagToken, remainTokens: TopLevelToken[], liquid: Liquid) {
     super(tagToken, remainTokens, liquid)
-    let p
+    let p: Template[] = []
+    let elseCount = 0
     this.liquid.parser.parseStream(remainTokens)
       .on('start', () => this.branches.push({
         value: new Value(tagToken.args, this.liquid),
         test: isFalsy,
         templates: (p = [])
       }))
-      .on('tag:elsif', (token: TagToken) => this.branches.push({
-        value: new Value(token.args, this.liquid),
-        test: isTruthy,
-        templates: (p = [])
-      }))
-      .on('tag:else', () => (p = this.elseTemplates))
+      .on('tag:elsif', (token: TagToken) => {
+        if (elseCount > 0) {
+          p = []
+          return
+        }
+        this.branches.push({
+          value: new Value(token.args, this.liquid),
+          test: isTruthy,
+          templates: (p = [])
+        })
+      })
+      .on('tag:else', () => {
+        elseCount++
+        p = this.elseTemplates
+      })
       .on('tag:endunless', function () { this.stop() })
-      .on('template', (tpl: Template) => p.push(tpl))
+      .on('template', (tpl: Template) => {
+        if (p !== this.elseTemplates || elseCount === 1) {
+          p.push(tpl)
+        }
+      })
       .on('end', () => { throw new Error(`tag ${tagToken.getText()} not closed`) })
       .start()
   }
diff --git a/test/e2e/issues.spec.ts b/test/e2e/issues.spec.ts
index a195b4919..a73257c99 100644
--- a/test/e2e/issues.spec.ts
+++ b/test/e2e/issues.spec.ts
@@ -469,4 +469,20 @@ describe('Issues', function () {
     const result = engine.parseAndRenderSync('{{ÜLKE}}', { ÜLKE: 'Türkiye' })
     expect(result).toEqual('Türkiye')
   })
+  it('#670 Should not render anything after an else branch', () => {
+    const engine = new Liquid()
+    const result = engine.parseAndRenderSync('{% assign value = "this" %}' +
+      '{% if false %}don\'t show' +
+      '{% else %}show {{ value }}' +
+    '{% else %}don\'t show{% endif %}', {})
+    expect(result).toEqual('show this')
+  })
+  it('#672 Should not render an elseif after an else branch', () => {
+    const engine = new Liquid()
+    const result = engine.parseAndRenderSync('{% if false %}don\'t show' +
+      '{% else %}show' +
+      '{% elsif true %}don\'t show' +
+    '{% endif %}', {})
+    expect(result).toEqual('show')
+  })
 })
diff --git a/test/integration/tags/case.spec.ts b/test/integration/tags/case.spec.ts
index 3312eb800..ce0d8f86f 100644
--- a/test/integration/tags/case.spec.ts
+++ b/test/integration/tags/case.spec.ts
@@ -91,4 +91,32 @@ describe('tags/case', function () {
     const html = await liquid.parseAndRender(src)
     return expect(html).toBe('and or or')
   })
+  it('should not render anything after an else branch', async function () {
+    const html = await liquid.parseAndRenderSync('{% assign value = "this" %}' +
+    '{% case true %}' +
+      '{% when false %}don\'t show' +
+      '{% else %}show {{ value }}' +
+      '{% else %}don\'t show' +
+    '{% endcase %}', {})
+    expect(html).toEqual('show this')
+  })
+  it('should not render anything after an else branch even when first else branch is empty', async function () {
+    const html = await liquid.parseAndRenderSync('{% case true %}' +
+      '{% when false %}don\'t show' +
+      '{% else %}' +
+      '{% else %}don\'t show' +
+    '{% endcase %}', {})
+    expect(html).toEqual('')
+  })
+  it('should not render anything after an else branch even when there are \'when\' conditions', () => {
+    const engine = new Liquid()
+    const result = engine.parseAndRenderSync('{% assign value = "this" %}' +
+    '{% case true -%}' +
+      '{% when false -%}don\'t show' +
+      '{% else %}show {{ value }}' +
+      '{% else %}don\'t show' +
+      '{%- when true -%}don\'t show' +
+    '{%- endcase %}', {})
+    expect(result).toEqual('show this')
+  })
 })
diff --git a/test/integration/tags/if.spec.ts b/test/integration/tags/if.spec.ts
index ba6c35fb1..2086b0360 100644
--- a/test/integration/tags/if.spec.ts
+++ b/test/integration/tags/if.spec.ts
@@ -143,4 +143,12 @@ describe('tags/if', function () {
     const html = await liquid.parseAndRender(src, scope)
     return expect(html).toBe('success')
   })
+  it('should not render anything after an else branch even when first else branch is empty', () => {
+    const engine = new Liquid()
+    const result = engine.parseAndRenderSync('{% if false %}don\'t show' +
+      '{% else %}' +
+      '{% else %}don\'t show' +
+    '%{% endif %}', {})
+    expect(result).toEqual('')
+  })
 })
diff --git a/test/integration/tags/unless.spec.ts b/test/integration/tags/unless.spec.ts
index f31175051..5ca0446a6 100644
--- a/test/integration/tags/unless.spec.ts
+++ b/test/integration/tags/unless.spec.ts
@@ -47,6 +47,29 @@ describe('tags/unless', function () {
       Inside wonderland
       After wonderland`)
   })
+  it('should not render anything after an else branch', async function () {
+    const html = await liquid.parseAndRenderSync('{% assign value = "this" %}' +
+      '{% unless true %}don\'t show' +
+      '{% else %}show {{ value }}' +
+      '{% else %}don\'t show' +
+    '{% endunless %}', {})
+    expect(html).toEqual('show this')
+  })
+  it('should not render anything after an else branch even when first else branch is empty', async function () {
+    const html = await liquid.parseAndRenderSync('{% unless true %}don\'t show' +
+      '{% else %}' +
+      '{% else %}don\'t show' +
+    '{% endunless %}', {})
+    expect(html).toEqual('')
+  })
+  it('should not render an elseif after an else branch', () => {
+    const engine = new Liquid()
+    const result = engine.parseAndRenderSync('{% unless true %}don\'t show' +
+      '{% else %}show' +
+      '{% elsif true %}don\'t show' +
+    '{% endunless %}', {})
+    expect(result).toEqual('show')
+  })
 
   describe('sync support', function () {
     it('should render else when predicate yields true', function () {