Fixed NullPointerException in SecureLogFilter.isLoggable

[nikosmoum]

Environment

Liquibase Version: 4.8.0

Liquibase Integration & Version: CLI

Liquibase Extension(s) & Version: none

Database Vendor & Version: I don't think it matters, but I tested with Postgres 12

Operating System Type & Version: all (I tested on Fedora35)

Pull Request Type

• Bug fix (non-breaking change which fixes an issue.)
• Enhancement/New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Fixes #1858

Steps To Reproduce

List the steps to reproduce the behavior.

• Create a changelog.xml that uses a custom task (that implements liquibase.change.custom.CustomTaskChange).
• In the custom task, override the getConfirmationMessage method to return null:

    @Override
    public String getConfirmationMessage() {
        return null;
    }

• Run the 'update' command from the CLI, using either the liquibase.integration.commandline.Main or liquibase.integration.commandline.LiquibaseCommandLine, and setting the --log-level= argument (it's value doesn't matter, could be info/debug/whatever)

Actual Behavior

The update fails. Here is the last part of the stacktrace:

Caused by: java.lang.NullPointerException
	at liquibase.integration.commandline.LiquibaseCommandLine$SecureLogFilter.isLoggable(LiquibaseCommandLine.java:1055)
	at java.logging/java.util.logging.Handler.isLoggable(Handler.java:346)
	at java.logging/java.util.logging.StreamHandler.isLoggable(StreamHandler.java:237)
	at java.logging/java.util.logging.StreamHandler.publish(StreamHandler.java:194)
	at java.logging/java.util.logging.ConsoleHandler.publish(ConsoleHandler.java:95)
	at java.logging/java.util.logging.Logger.log(Logger.java:979)
	at java.logging/java.util.logging.Logger.doLog(Logger.java:1006)
	at java.logging/java.util.logging.Logger.logp(Logger.java:1283)
	at liquibase.logging.core.JavaLogger.log(JavaLogger.java:30)
	at liquibase.logging.core.CompositeLogger.log(CompositeLogger.java:33)
	at liquibase.logging.core.CompositeLogger.log(CompositeLogger.java:33)
	at liquibase.logging.core.AbstractLogger.info(AbstractLogger.java:50)
	at liquibase.logging.core.AbstractLogger.info(AbstractLogger.java:45)
	at liquibase.changelog.ChangeSet.execute(ChangeSet.java:662)
	... 62 more

Expected/Desired Behavior

The update should succeed.

Fast Track PR Acceptance Checklist:

• Build is successful and all new and existing tests pass
• Added Unit Test(s)
• Added Integration Test(s)
• Added Test Harness Test(s)
• Documentation Updated

Need Help?

Come chat with us on our discord channel

[github-actions]

Unit Test Results

  4 512 files  ±  0    4 512 suites  ±0   42m 4s ⏱️ + 7m 19s
  4 376 tests +  2    4 158 ✔️  -   2     218 💤 +4  0 ❌ ±0 
51 792 runs  +24  46 780 ✔️ +20  5 012 💤 +4  0 ❌ ±0 

Results for commit efe3380. ± Comparison against base commit 97e6821.

♻️ This comment has been updated with latest results.

[nikosmoum]

@nvoxland I don't have access to the "Stable Liquibase Pro" build, so I don't know why/what failed. Is it something I need to fix on my side? Also, the "Package Artifacts" failed due to some kind of infrastructure issue.

[nvoxland]

The SafeToBuild label is something we have to add based on reviewing the code. It's a temporary solution as we're fixing up the build logic to be more secure

[XDelphiGrl]

@nvoxland, this build did not run, despite having the SafeToBuild label. Can you take a look please?

[nvoxland]

#2777 removes this class completely, so if that PR gets merged first this can be closed as "unneeded"

[XDelphiGrl]

@nvoxland, can you please confirm that we want to swap out piccoli.CommandLine in favor of using liquibase.logging.LogMessageFilter? I ask because there are plans to deprecate the LogMessageFilter class.

CC @nikosmoum

[nvoxland]

That is the right class to be importing, and also part of why this PR won't be needed if/when #2777 goes through. I just hadn't closed this one yet since whether we want to do this or not will depend on what happens with 2777

[XDelphiGrl]

• Fix is focused on addressing a Null Pointer.
• New integration and unit tests added.
• No further testing required.

Passing Test Harness Execution
Passing Functional Tests

APPROVED