From 32c39614f56caa0368c1583813e7bad14e3ad758 Mon Sep 17 00:00:00 2001
From: Daniel Gray <the.daniel.gray@gmail.com>
Date: Thu, 6 May 2021 18:29:50 +0200
Subject: [PATCH] Prevent silent failures when two "databaseChangeLog" tags are
 present

If someone makes a mistake and copy/pastes a databaseChangeLog block in,
and makes the file have two tags, the SnakeYAML parser silently covers
up the first instance with the second one, which will cause the first
block of changes to be silently omitted.

This can be very problematic and cause hard-to-debug errors, which is
why I have added a small FilterInputStream interceptor to check the
stream as it is fed into the YAML parser, for a duplicate tag. Doing it
with the FilterInputStream like this prevents the efficiency losses of
opening the file twice.
---
 .../parser/core/yaml/YamlChangeLogParser.java | 71 ++++++++++++++++++-
 .../YamlChangeLogParser_RealFile_Test.groovy  |  9 +++
 .../core/yaml/malformedDoubleChangeLog.yaml   | 46 ++++++++++++
 .../dbtest/AbstractIntegrationTest.java       |  1 +
 4 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100644 liquibase-core/src/test/resources/liquibase/parser/core/yaml/malformedDoubleChangeLog.yaml

diff --git a/liquibase-core/src/main/java/liquibase/parser/core/yaml/YamlChangeLogParser.java b/liquibase-core/src/main/java/liquibase/parser/core/yaml/YamlChangeLogParser.java
index b5411d8bfbb..c131fa6de20 100644
--- a/liquibase-core/src/main/java/liquibase/parser/core/yaml/YamlChangeLogParser.java
+++ b/liquibase-core/src/main/java/liquibase/parser/core/yaml/YamlChangeLogParser.java
@@ -13,8 +13,10 @@
 import org.yaml.snakeyaml.Yaml;
 import org.yaml.snakeyaml.constructor.SafeConstructor;
 
+import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 import java.util.*;
 
 public class YamlChangeLogParser extends YamlParser implements ChangeLogParser {
@@ -83,11 +85,78 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame
             throw new ChangeLogParseException("Error parsing "+physicalChangeLogLocation, e);
         }
     }
+
+    /**
+     * A simple "interceptor" InputStream to ensure that the YAML stream doesn't contain two "databaseChangeLog" tags,
+     * which would otherwise cause silent omissions if something is copy/pasted wrongly into the file.
+     */
+    static class DoubleChangelogCheckerInputStream extends FilterInputStream {
+
+        byte[] databaseChangeLog = "databaseChangeLog".getBytes(StandardCharsets.UTF_8);
+        int scanPos;
+        boolean databaseChangeLogSeen;
+
+        DoubleChangelogCheckerInputStream(InputStream in) {
+            super(in);
+            scanPos = 0;
+            databaseChangeLogSeen = false;
+        }
+
+        private void processByte(int nextByte) {
+            // scanPos will be -1 if we aren't in a position where "databaseChangeLog" would cause a problem
+            // i.e. the beginning of a file or of a line
+            if (nextByte == 10) {
+                // a new line means we should expect to see it again
+                scanPos = 0;
+            } else if (scanPos > -1 && nextByte == databaseChangeLog[scanPos]) {
+                scanPos++;
+                if (scanPos == databaseChangeLog.length) {
+                    if (databaseChangeLogSeen) {
+                        throw new IllegalStateException("Two 'databaseChangeLog' tags found in a single file!");
+                    } else {
+                        databaseChangeLogSeen = true;
+                        scanPos = -1;
+                    }
+                }
+            } else {
+                // no point in scanning since the string was not found
+                scanPos = -1;
+            }
+        }
+
+        @Override
+        public int read() throws IOException {
+            int nextByte = super.read();
+            processByte(nextByte);
+            return nextByte;
+        }
+
+        @Override
+        public int read(byte[] b) throws IOException {
+            int numBytes = super.read(b);
+
+            for (int i=0; i<numBytes; i++) {
+                processByte(b[i]);
+            }
+
+            return numBytes;
+        }
+
+        @Override
+		public int read(byte[] b, int off, int len) throws IOException {
+            int numBytes = super.read(b, off, len);
+
+            for (int i=off; i < off + numBytes; i++) {
+                processByte(b[i]);
+            }
+            return numBytes;
+        }
+    }
     
     private Map parseYamlStream(String physicalChangeLogLocation, Yaml yaml, InputStream changeLogStream) throws ChangeLogParseException {
         Map parsedYaml;
         try {
-            parsedYaml = (Map) yaml.load(changeLogStream);
+            parsedYaml = (Map) yaml.load(new DoubleChangelogCheckerInputStream(changeLogStream));
         } catch (Exception e) {
             throw new ChangeLogParseException("Syntax error in file " + physicalChangeLogLocation + ": " + e.getMessage(), e);
         }
diff --git a/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy b/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy
index deb2394ed18..519c8ee5b49 100644
--- a/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy
+++ b/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy
@@ -290,6 +290,15 @@ public class YamlChangeLogParser_RealFile_Test extends Specification {
         assert e.message.startsWith("Syntax error in file liquibase/parser/core/yaml/malformedChangeLog.yaml")
     }
 
+    def "ChangeLogParseException thrown if changelog has two databaseChangeLog tags"() throws Exception {
+        when:
+        new YamlChangeLogParser().parse("liquibase/parser/core/yaml/malformedDoubleChangeLog.yaml", new ChangeLogParameters(), new JUnitResourceAccessor())
+
+        then:
+        def e = thrown(ChangeLogParseException)
+        assert e.message.startsWith("Syntax error in file liquibase/parser/core/yaml/malformedDoubleChangeLog.yaml")
+    }
+
     def "elements that don't correspond to anything in liquibase are ignored"() throws Exception {
         def path = "liquibase/parser/core/yaml/unusedTagsChangeLog.yaml"
         expect:
diff --git a/liquibase-core/src/test/resources/liquibase/parser/core/yaml/malformedDoubleChangeLog.yaml b/liquibase-core/src/test/resources/liquibase/parser/core/yaml/malformedDoubleChangeLog.yaml
new file mode 100644
index 00000000000..916e4abea0e
--- /dev/null
+++ b/liquibase-core/src/test/resources/liquibase/parser/core/yaml/malformedDoubleChangeLog.yaml
@@ -0,0 +1,46 @@
+databaseChangeLog:
+    - changeSet:
+          id: 1
+          author: nvoxland
+          comment: "Some comments go here"
+          changes:
+              - createTable:
+                    tableName: person_yaml
+                    columns:
+                        - column:
+                              name: id
+                              type: int
+                              constraints:
+                                  primaryKey: true
+                                  nullable: false
+                        - column:
+                              name: name
+                              type: varchar(255)
+                              constraints:
+                                  nullable: false
+                        - column:
+                              name: age
+                              type: int
+databaseChangeLog:
+    - changeSet:
+          id: 2
+          author: nvoxland
+          comment: "Some other comments go here"
+          changes:
+              - createTable:
+                    tableName: person_yaml2
+                    columns:
+                        - column:
+                              name: id
+                              type: int
+                              constraints:
+                                  primaryKey: true
+                                  nullable: false
+                        - column:
+                              name: name
+                              type: varchar(255)
+                              constraints:
+                                  nullable: false
+                        - column:
+                              name: age
+                              type: int
diff --git a/liquibase-integration-tests/src/test/java/liquibase/dbtest/AbstractIntegrationTest.java b/liquibase-integration-tests/src/test/java/liquibase/dbtest/AbstractIntegrationTest.java
index 9a894966c22..7520e1ac81c 100644
--- a/liquibase-integration-tests/src/test/java/liquibase/dbtest/AbstractIntegrationTest.java
+++ b/liquibase-integration-tests/src/test/java/liquibase/dbtest/AbstractIntegrationTest.java
@@ -788,6 +788,7 @@ public void testRerunDiffChangeLogAltSchema() throws Exception {
         DiffResult diffResult = DiffGeneratorFactory.getInstance().compare(database, null, compareControl);
 
         File tempFile = File.createTempFile("liquibase-test", ".xml");
+        tempFile.deleteOnExit();
 
         FileOutputStream output = new FileOutputStream(tempFile);
         try {