Skip to content

Commit

Permalink
Prevent silent failures when two "databaseChangeLog" tags are present
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
danielthegray committed May 6, 2021
1 parent c95798b commit 32c3961
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 1 deletion.
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down
Expand Up @@ -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:
Expand Down
@@ -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
Expand Up @@ -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 {
Expand Down

0 comments on commit 32c3961

Please sign in to comment.