diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java index 11bbccb8c54..5d09c11156b 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java @@ -634,6 +634,7 @@ public static class Rfc5424LayoutBuilder extends AbstractStringLayout.Builder BUILDER_ATTRIBUTE_ANNOTATIONS = List.of( + "org.apache.logging.log4j.plugins.PluginBuilderAttribute", + "org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute"); + private static final String SERVICE_FILE_NAME = "META-INF/services/org.apache.logging.log4j.plugins.model.PluginService"; @@ -121,9 +132,13 @@ public synchronized void init(ProcessingEnvironment processingEnv) { @Override public boolean process(final Set annotations, final RoundEnvironment roundEnv) { // Process the elements for this round - if (!annotations.isEmpty()) { - processPluginAnnotatedClasses(ElementFilter.typesIn(roundEnv.getElementsAnnotatedWith(Plugin.class))); + for (TypeElement annotation : annotations) { + if (annotation.getQualifiedName().contentEquals("org.apache.logging.log4j.plugins.Plugin")) { + processPluginAnnotatedClasses(ElementFilter.typesIn(roundEnv.getElementsAnnotatedWith(annotation))); + } } + // Validate @PluginBuilderAttribute fields in builder classes + processBuilderAttributeFields(roundEnv); // Write the generated code if (roundEnv.processingOver() && !pluginIndex.isEmpty()) { try { @@ -164,6 +179,76 @@ private void processPluginAnnotatedClasses(Set pluginClasses) { } } + private void processBuilderAttributeFields(final RoundEnvironment roundEnv) { + final Elements elements = processingEnv.getElementUtils(); + for (final String annotationFqn : BUILDER_ATTRIBUTE_ANNOTATIONS) { + final TypeElement annotationType = elements.getTypeElement(annotationFqn); + if (annotationType == null) { + continue; + } + for (final Element element : roundEnv.getElementsAnnotatedWith(annotationType)) { + if (element instanceof VariableElement) { + processBuilderAttributeField((VariableElement) element); + } + } + } + } + + private void processBuilderAttributeField(final VariableElement element) { + final String fieldName = element.getSimpleName().toString(); + // Check for @SuppressWarnings("log4j.public.setter") + final SuppressWarnings suppress = element.getAnnotation(SuppressWarnings.class); + if (suppress != null && Arrays.asList(suppress.value()).contains(SUPPRESS_WARNING_PUBLIC_SETTER)) { + return; + } + final Element enclosingElement = element.getEnclosingElement(); + // element is a field, its enclosing element is a type + if (enclosingElement instanceof TypeElement) { + final TypeElement typeElement = (TypeElement) enclosingElement; + // Check the siblings of the field for a matching setter + for (final Element enclosedElement : typeElement.getEnclosedElements()) { + if (enclosedElement instanceof ExecutableElement) { + final ExecutableElement methodElement = (ExecutableElement) enclosedElement; + final String methodName = methodElement.getSimpleName().toString(); + if ((methodName.toLowerCase(Locale.ROOT).startsWith("set") + || methodName.toLowerCase(Locale.ROOT).startsWith("with")) + && methodElement.getParameters().size() == 1) { + final Types typeUtils = processingEnv.getTypeUtils(); + final boolean followsNamePattern = methodName.equals( + String.format("set%s", expectedFieldNameInASetter(fieldName))) + || methodName.equals(String.format("with%s", expectedFieldNameInASetter(fieldName))); + final boolean isPublicMethod = + methodElement.getModifiers().contains(Modifier.PUBLIC); + final boolean checkForAssignable = typeUtils.isAssignable( + methodElement.getReturnType(), + methodElement.getEnclosingElement().asType()); + final boolean foundPublicSetter = followsNamePattern && checkForAssignable && isPublicMethod; + if (foundPublicSetter) { + return; + } + } + } + } + // No setter found: emit a compilation error + processingEnv + .getMessager() + .printMessage( + javax.tools.Diagnostic.Kind.ERROR, + String.format( + "The field `%s` does not have a public setter. " + + "Note that @SuppressWarnings(\"%s\") can be used on the field to suppress this error.", + fieldName, SUPPRESS_WARNING_PUBLIC_SETTER), + element); + } + } + + private static String expectedFieldNameInASetter(String fieldName) { + if (fieldName.startsWith("is")) { + fieldName = fieldName.substring(2); + } + return fieldName.isEmpty() ? fieldName : Character.toUpperCase(fieldName.charAt(0)) + fieldName.substring(1); + } + private static void processConfigurableAnnotation(TypeElement pluginClass, PluginEntry.Builder builder) { var configurable = pluginClass.getAnnotation(Configurable.class); if (configurable != null) { diff --git a/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java b/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java new file mode 100644 index 00000000000..ddd9db1157b --- /dev/null +++ b/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java @@ -0,0 +1,122 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you 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 + * + * http://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.apache.logging.log4j.plugin.processor; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import java.util.stream.Collectors; +import javax.tools.Diagnostic; +import javax.tools.DiagnosticCollector; +import javax.tools.JavaCompiler; +import javax.tools.JavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.StandardLocation; +import javax.tools.ToolProvider; +import org.apache.commons.io.FileUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +public class PluginProcessorPublicSetterTest { + + private static final String FAKE_PLUGIN_CLASS_PATH = + "src/test/resources/setter-test/FakePluginPublicSetter.java.source"; + + @TempDir + Path outputDir; + + private File createdFile; + private DiagnosticCollector diagnosticCollector; + private List> errorDiagnostics; + + @BeforeEach + void setup() { + final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + diagnosticCollector = new DiagnosticCollector<>(); + final StandardJavaFileManager fileManager = compiler.getStandardFileManager(null, Locale.ROOT, UTF_8); + + final Path sourceFile = Paths.get(FAKE_PLUGIN_CLASS_PATH); + + assertThat(sourceFile).exists(); + + final File orig = sourceFile.toFile(); + createdFile = new File(orig.getParentFile(), "FakePluginPublicSetter.java"); + assertDoesNotThrow(() -> FileUtils.copyFile(orig, createdFile)); + + final Iterable compilationUnits = fileManager.getJavaFileObjects(createdFile); + + // Route generated files (Log4jPlugins.java, META-INF/services/...) to the temp directory + final File outputDirFile = outputDir.toFile(); + try { + fileManager.setLocation(StandardLocation.CLASS_OUTPUT, Set.of(outputDirFile)); + fileManager.setLocation(StandardLocation.SOURCE_OUTPUT, Set.of(outputDirFile)); + } catch (final IOException e) { + throw new RuntimeException("Failed to set output location", e); + } + + final JavaCompiler.CompilationTask task = compiler.getTask( + null, + fileManager, + diagnosticCollector, + Arrays.asList("-proc:only", "-processor", PluginProcessor.class.getName()), + null, + compilationUnits); + task.call(); + + errorDiagnostics = diagnosticCollector.getDiagnostics().stream() + .filter(diagnostic -> diagnostic.getKind() == Diagnostic.Kind.ERROR) + .collect(Collectors.toList()); + } + + @AfterEach + void tearDown() { + assertDoesNotThrow(() -> FileUtils.delete(createdFile)); + } + + @Test + void warnWhenPluginBuilderAttributeLacksPublicSetter() { + assertThat(errorDiagnostics).anyMatch(errorMessage -> errorMessage + .getMessage(Locale.ROOT) + .contains("The field `attributeWithoutPublicSetter` does not have a public setter")); + } + + @Test + void ignoreWarningWhenSuppressWarningsIsPresent() { + assertThat(errorDiagnostics).allMatch(errorMessage -> !errorMessage + .getMessage(Locale.ROOT) + .contains("The field `attributeWithoutPublicSetterButWithSuppressAnnotation`" + + " does not have a public setter")); + } + + @Test + void noWarningWhenPublicSetterExists() { + assertThat(errorDiagnostics).allMatch(errorMessage -> !errorMessage + .getMessage(Locale.ROOT) + .contains("The field `attribute` does not have a public setter")); + } +} diff --git a/log4j-plugin-processor/src/test/resources/setter-test/FakePluginPublicSetter.java.source b/log4j-plugin-processor/src/test/resources/setter-test/FakePluginPublicSetter.java.source new file mode 100644 index 00000000000..6a7b9c12391 --- /dev/null +++ b/log4j-plugin-processor/src/test/resources/setter-test/FakePluginPublicSetter.java.source @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you 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 + * + * http://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 example; + +import org.apache.logging.log4j.plugins.Plugin; +import org.apache.logging.log4j.plugins.PluginBuilderAttribute; + +/** + * Test plugin class for unit tests of public setter validation. + */ +@Plugin("FakePluginPublicSetter") +public class FakePluginPublicSetter { + + public static class Builder { + + @PluginBuilderAttribute + private int attribute; + + @PluginBuilderAttribute + @SuppressWarnings("log4j.public.setter") + private int attributeWithoutPublicSetterButWithSuppressAnnotation; + + @PluginBuilderAttribute + private int attributeWithoutPublicSetter; + + public Builder setAttribute(final int attribute) { + this.attribute = attribute; + return this; + } + + public Builder setAttributeWithoutPublicSetterButWithSuppressAnnotation( + final int attributeWithoutPublicSetterButWithSuppressAnnotation) { + this.attributeWithoutPublicSetterButWithSuppressAnnotation = + attributeWithoutPublicSetterButWithSuppressAnnotation; + return this; + } + } +}