Skip to content

Commit

Permalink
Warning if <springProfile> is used in phase 2 model elements
Browse files Browse the repository at this point in the history
Add `SpringProfileIfNestedWithinSecondPhaseElementSanityChecker` which
will provide a warning if `<springProfile>` is used within a phase 2
model element. This is similar to Logback's own `<if>` warnings.

The `LogbackLoggingSystem` has also been updated so that warning are
printed when present.

Fixes gh-33610
  • Loading branch information
philwebb committed Dec 22, 2022
1 parent 2ed512d commit 2e7ca6f
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
import ch.qos.logback.core.spi.FilterReply;
import ch.qos.logback.core.status.OnConsoleStatusListener;
import ch.qos.logback.core.status.Status;
import ch.qos.logback.core.status.StatusUtil;
import ch.qos.logback.core.util.StatusListenerConfigHelper;
import ch.qos.logback.core.util.StatusPrinter;
import org.slf4j.ILoggerFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -250,6 +252,9 @@ protected void loadConfiguration(LoggingInitializationContext initializationCont
if (errors.length() > 0) {
throw new IllegalStateException(String.format("Logback configuration error detected: %n%s", errors));
}
if (!StatusUtil.contextHasStatusListener(loggerContext)) {
StatusPrinter.printInCaseOfErrorsOrWarnings(loggerContext);
}
}

private void configureByResourceUrl(LoggingInitializationContext initializationContext, LoggerContext loggerContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ class SpringBootJoranConfigurator extends JoranConfigurator {
this.initializationContext = initializationContext;
}

@Override
protected void sanityCheck(Model topModel) {
super.sanityCheck(topModel);
performCheck(new SpringProfileIfNestedWithinSecondPhaseElementSanityChecker(), topModel);
}

@Override
protected void addModelHandlerAssociations(DefaultProcessor defaultProcessor) {
defaultProcessor.addHandler(SpringPropertyModel.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright 2012-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.boot.logging.logback;

import java.util.ArrayList;
import java.util.List;

import ch.qos.logback.classic.joran.sanity.IfNestedWithinSecondPhaseElementSC;
import ch.qos.logback.classic.model.LoggerModel;
import ch.qos.logback.classic.model.RootLoggerModel;
import ch.qos.logback.core.joran.sanity.Pair;
import ch.qos.logback.core.joran.sanity.SanityChecker;
import ch.qos.logback.core.model.AppenderModel;
import ch.qos.logback.core.model.Model;
import ch.qos.logback.core.spi.ContextAwareBase;

/**
* {@link SanityChecker} to ensure that {@code springProfile} elements are not nested
* within second-phase elements.
*
* @author Phillip Webb
* @see IfNestedWithinSecondPhaseElementSC
*/
class SpringProfileIfNestedWithinSecondPhaseElementSanityChecker extends ContextAwareBase implements SanityChecker {

private static final List<Class<? extends Model>> SECOND_PHASE_TYPES = List.of(AppenderModel.class,
LoggerModel.class, RootLoggerModel.class);

@Override
public void check(Model model) {
if (model == null) {
return;
}
List<Model> models = new ArrayList<>();
SECOND_PHASE_TYPES.forEach((type) -> deepFindAllModelsOfType(type, models, model));
List<Pair<Model, Model>> nestedPairs = deepFindNestedSubModelsOfType(SpringProfileModel.class, models);
if (!nestedPairs.isEmpty()) {
addWarn("<springProfile> elements cannot be nested within an <appender>, <logger> or <root> element");
nestedPairs.forEach((nested) -> {
Model first = nested.first;
Model second = nested.second;
addWarn("Element <%s> at line %s contains a nested <%s> element at line %s".formatted(first.getTag(),
first.getLineNumber(), second.getTag(), second.getLineNumber()));
});
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import ch.qos.logback.core.encoder.LayoutWrappingEncoder;
import ch.qos.logback.core.rolling.RollingFileAppender;
import ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy;
import ch.qos.logback.core.util.StatusPrinter;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -110,6 +111,7 @@ void setup() {
ConversionService conversionService = ApplicationConversionService.getSharedInstance();
this.environment.setConversionService((ConfigurableConversionService) conversionService);
this.initializationContext = new LoggingInitializationContext(this.environment);
StatusPrinter.setPrintStream(System.out);
}

@AfterEach
Expand Down Expand Up @@ -665,6 +667,14 @@ void whenContextHasAotContributionThenProcessAheadOfTimeClearsAndReturnsIt() {
assertThat(contribution).isNotNull();
}

@Test // gh-33610
void springProfileIfNestedWithinSecondPhaseElementSanityChecker(CapturedOutput output) {
this.loggingSystem.beforeInitialize();
initialize(this.initializationContext, "classpath:logback-springprofile-in-root.xml", null);
this.logger.info("Hello world");
assertThat(output).contains("<springProfile> elements cannot be nested within an");
}

private void initialize(LoggingInitializationContext context, String configLocation, LogFile logFile) {
this.loggingSystem.getSystemProperties((ConfigurableEnvironment) context.getEnvironment()).apply(logFile);
this.loggingSystem.initialize(context, configLocation, logFile);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>
<configuration>
<appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
<encoder>
<pattern>%property{LOG_FILE} [%t] ${PID:-????} %c{1}: %m%n BOOTBOOT</pattern>
</encoder>
</appender>
<root level="INFO">
<springProfile name="profile">
<appender-ref ref="CONSOLE" />
</springProfile>
</root>
</configuration>

0 comments on commit 2e7ca6f

Please sign in to comment.