From 19a80cf5fa6afbfae615bed6a7fe051f8368ae2d Mon Sep 17 00:00:00 2001 From: phymbert Date: Wed, 21 Jun 2017 16:22:07 +0300 Subject: [LOG4J2-1699] Fix createOnDemand=false + PosixViewAttributeAction --- .../logging/log4j/core/appender/FileManager.java | 83 ++++--- .../appender/rolling/AbstractRolloverStrategy.java | 6 - .../appender/rolling/DefaultRolloverStrategy.java | 21 +- .../rolling/DirectWriteRolloverStrategy.java | 24 +- .../core/appender/rolling/RollingFileManager.java | 11 +- .../rolling/RollingRandomAccessFileManager.java | 6 +- .../rolling/action/PosixViewAttributeAction.java | 173 ++++++++++++++ .../apache/logging/log4j/core/util/FileUtils.java | 49 ++++ .../log4j/core/appender/FilePermissionsTest.java | 256 +++++++++++++++------ ...RollingAppenderSizeCompressPermissionsTest.java | 103 +++++++++ log4j-core/src/test/resources/log4j-posix.xml | 14 ++ .../src/test/resources/log4j-rolling-gz-posix.xml | 70 ++++++ 12 files changed, 700 insertions(+), 116 deletions(-) create mode 100644 log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java create mode 100644 log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderSizeCompressPermissionsTest.java create mode 100644 log4j-core/src/test/resources/log4j-posix.xml create mode 100644 log4j-core/src/test/resources/log4j-rolling-gz-posix.xml diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java index 511965d..2438e97 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java @@ -25,15 +25,12 @@ import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.nio.channels.FileLock; import java.nio.file.FileSystems; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.FileOwnerAttributeView; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFilePermission; import java.nio.file.attribute.PosixFilePermissions; -import java.nio.file.attribute.UserPrincipal; -import java.nio.file.attribute.UserPrincipalLookupService; import java.util.Date; import java.util.HashMap; import java.util.Map; @@ -61,6 +58,7 @@ public class FileManager extends OutputStreamManager { private final Set filePermissions; private final String fileOwner; private final String fileGroup; + private final boolean posixSupported; /** * @deprecated @@ -89,6 +87,7 @@ public class FileManager extends OutputStreamManager { this.filePermissions = null; this.fileOwner = null; this.fileGroup = null; + this.posixSupported = false; } /** @@ -108,6 +107,7 @@ public class FileManager extends OutputStreamManager { this.filePermissions = null; this.fileOwner = null; this.fileGroup = null; + this.posixSupported = false; } /** @@ -128,7 +128,10 @@ public class FileManager extends OutputStreamManager { this.filePermissions = filePermissions != null && views.contains("posix") ? PosixFilePermissions.fromString(filePermissions) : null; this.fileOwner = views.contains("owner") ? fileOwner : null; - this.fileGroup = views.contains("group") ? fileGroup : null; + this.fileGroup = views.contains("posix") ? fileGroup : null; + + // Supported and defined + this.posixSupported = filePermissions != null || fileOwner != null || fileGroup != null; } /** @@ -170,33 +173,15 @@ public class FileManager extends OutputStreamManager { } protected void definePathAttributeView(final Path path) throws IOException { - if (filePermissions != null || fileOwner != null || fileGroup != null) { + if (posixSupported) { // FileOutputStream may not create new file on all jvm path.toFile().createNewFile(); - final PosixFileAttributeView view = Files.getFileAttributeView(path, PosixFileAttributeView.class); - if (view != null) { - final UserPrincipalLookupService lookupService = FileSystems.getDefault() - .getUserPrincipalLookupService(); - if (fileOwner != null) { - final UserPrincipal userPrincipal = lookupService.lookupPrincipalByName(fileOwner); - if (userPrincipal != null) { - view.setOwner(userPrincipal); - } - } - if (fileGroup != null) { - final GroupPrincipal groupPrincipal = lookupService.lookupPrincipalByGroupName(fileGroup); - if (groupPrincipal != null) { - view.setGroup(groupPrincipal); - } - } - if (filePermissions != null) { - view.setPermissions(filePermissions); - } - } + FileUtils.defineFilePosixAttributeView(path, filePermissions, fileOwner, fileGroup); } } + @Override protected synchronized void write(final byte[] bytes, final int offset, final int length, final boolean immediateFlush) { @@ -263,7 +248,6 @@ public class FileManager extends OutputStreamManager { public String getFileName() { return getName(); } - /** * Returns the append status. * @return true if the file will be appended to, false if it is overwritten. @@ -296,6 +280,45 @@ public class FileManager extends OutputStreamManager { public int getBufferSize() { return bufferSize; } + + /** + * Returns posix file permissions if defined and the OS supports posix file attribute, + * null otherwise. + * @return File posix permissions + * @see PosixFileAttributeView + */ + public Set getFilePermissions() { + return filePermissions; + } + + /** + * Returns file owner if defined and the OS supports owner file attribute view, + * null otherwise. + * @return File owner + * @see FileOwnerAttributeView + */ + public String getFileOwner() { + return fileOwner; + } + + /** + * Returns file group if defined and the OS supports posix/group file attribute view, + * null otherwise. + * @return File group + * @see PosixFileAttributeView + */ + public String getFileGroup() { + return fileGroup; + } + + /** + * If posix file attribute view supported and defined. + * + * @return True if posix supported and defined false otherwise. + */ + public boolean isPosixSupported() { + return posixSupported; + } /** * FileManager's content format is specified by: Key: "fileURI" Value: provided "advertiseURI" param. @@ -376,9 +399,13 @@ public class FileManager extends OutputStreamManager { final int actualSize = data.bufferedIo ? data.bufferSize : Constants.ENCODER_BYTE_BUFFER_SIZE; final ByteBuffer byteBuffer = ByteBuffer.wrap(new byte[actualSize]); final FileOutputStream fos = data.createOnDemand ? null : new FileOutputStream(file, data.append); - return new FileManager(data.getLoggerContext(), name, fos, data.append, data.locking, + FileManager fm = new FileManager(data.getLoggerContext(), name, fos, data.append, data.locking, data.createOnDemand, data.advertiseURI, data.layout, data.filePermissions, data.fileOwner, data.fileGroup, writeHeader, byteBuffer); + if (fos != null && fm.posixSupported) { + fm.definePathAttributeView(file.toPath()); + } + return fm; } catch (final IOException ex) { LOGGER.error("FileManager (" + name + ") " + ex, ex); } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/AbstractRolloverStrategy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/AbstractRolloverStrategy.java index 0179e80..5603e7c 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/AbstractRolloverStrategy.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/AbstractRolloverStrategy.java @@ -22,10 +22,7 @@ import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import java.util.Objects; import java.util.SortedMap; import java.util.TreeMap; import java.util.regex.Matcher; @@ -34,10 +31,7 @@ import java.util.regex.Pattern; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LoggingException; import org.apache.logging.log4j.core.appender.rolling.action.Action; -import org.apache.logging.log4j.core.appender.rolling.action.CommonsCompressAction; import org.apache.logging.log4j.core.appender.rolling.action.CompositeAction; -import org.apache.logging.log4j.core.appender.rolling.action.GzCompressAction; -import org.apache.logging.log4j.core.appender.rolling.action.ZipCompressAction; import org.apache.logging.log4j.core.lookup.StrSubstitutor; import org.apache.logging.log4j.core.pattern.NotANumber; import org.apache.logging.log4j.status.StatusLogger; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java index 6ffd54c..a91a453 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java @@ -20,7 +20,6 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -31,7 +30,10 @@ import java.util.zip.Deflater; import org.apache.logging.log4j.core.Core; import org.apache.logging.log4j.core.appender.rolling.action.Action; +import org.apache.logging.log4j.core.appender.rolling.action.CompositeAction; import org.apache.logging.log4j.core.appender.rolling.action.FileRenameAction; +import org.apache.logging.log4j.core.appender.rolling.action.PathCondition; +import org.apache.logging.log4j.core.appender.rolling.action.PosixViewAttributeAction; import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginAttribute; @@ -352,6 +354,23 @@ public class DefaultRolloverStrategy extends AbstractRolloverStrategy { return new RolloverDescriptionImpl(currentFileName, false, null, null); } + if (manager.isPosixSupported()) { + // Propagate posix attribute view to rolled/compressed file + Action posixAttributeViewAction = new PosixViewAttributeAction(compressedName, + false, + 1, + new PathCondition[0], + getStrSubstitutor(), + manager.getFilePermissions(), + manager.getFileOwner(), + manager.getFileGroup()); + if (compressAction == null) { + compressAction = posixAttributeViewAction; + } else { + compressAction = new CompositeAction(Arrays.asList(compressAction, posixAttributeViewAction), false); + } + } + final FileRenameAction renameAction = new FileRenameAction(new File(currentFileName), new File(renameTo), manager.isRenameEmptyFiles()); diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DirectWriteRolloverStrategy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DirectWriteRolloverStrategy.java index 92e9cca..feff516 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DirectWriteRolloverStrategy.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DirectWriteRolloverStrategy.java @@ -28,6 +28,9 @@ import java.util.zip.Deflater; import org.apache.logging.log4j.core.Core; import org.apache.logging.log4j.core.appender.rolling.action.Action; +import org.apache.logging.log4j.core.appender.rolling.action.CompositeAction; +import org.apache.logging.log4j.core.appender.rolling.action.PathCondition; +import org.apache.logging.log4j.core.appender.rolling.action.PosixViewAttributeAction; import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginAttribute; @@ -182,14 +185,33 @@ public class DirectWriteRolloverStrategy extends AbstractRolloverStrategy implem } Action compressAction = null; final String sourceName = currentFileName; + String compressedName = sourceName; currentFileName = null; nextIndex = fileIndex + 1; FileExtension fileExtension = manager.getFileExtension(); if (fileExtension != null) { - compressAction = fileExtension.createCompressAction(sourceName, sourceName + fileExtension.getExtension(), + compressedName += fileExtension.getExtension(); + compressAction = fileExtension.createCompressAction(sourceName, compressedName, true, compressionLevel); } + if (manager.isPosixSupported()) { + // Propagate posix attribute view to rolled/compressed file + Action posixAttributeViewAction = new PosixViewAttributeAction(compressedName, + false, + 1, + new PathCondition[0], + getStrSubstitutor(), + manager.getFilePermissions(), + manager.getFileOwner(), + manager.getFileGroup()); + if (compressAction == null) { + compressAction = posixAttributeViewAction; + } else { + compressAction = new CompositeAction(Arrays.asList(compressAction, posixAttributeViewAction), false); + } + } + final Action asyncAction = merge(compressAction, customActions, stopCustomActionsOnError); return new RolloverDescriptionImpl(sourceName, false, null, asyncAction); } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java index e0d0360..90b19ef 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java @@ -628,9 +628,14 @@ public class RollingFileManager extends FileManager { final long time = data.createOnDemand || file == null ? System.currentTimeMillis() : file.lastModified(); // LOG4J2-531 create file first so time has valid value - return new RollingFileManager(data.getLoggerContext(), data.fileName, data.pattern, os, - data.append, data.createOnDemand, size, time, data.policy, data.strategy, data.advertiseURI, - data.layout, data.filePermissions, data.fileOwner, data.fileGroup, writeHeader, buffer); + RollingFileManager rm = new RollingFileManager(data.getLoggerContext(), data.fileName, data.pattern, os, + data.append, data.createOnDemand, size, time, data.policy, data.strategy, data.advertiseURI, + data.layout, data.filePermissions, data.fileOwner, data.fileGroup, writeHeader, buffer); + if (os != null && rm.isPosixSupported()) { + rm.definePathAttributeView(file.toPath()); + } + + return rm; } catch (final IOException ex) { LOGGER.error("RollingFileManager (" + name + ") " + ex, ex); } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingRandomAccessFileManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingRandomAccessFileManager.java index f210bf5..ccce93c 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingRandomAccessFileManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingRandomAccessFileManager.java @@ -204,9 +204,13 @@ public class RollingRandomAccessFileManager extends RollingFileManager { LOGGER.trace("RandomAccessFile {} set length to 0", name); raf.setLength(0); } - return new RollingRandomAccessFileManager(data.getLoggerContext(), raf, name, data.pattern, + RollingRandomAccessFileManager rrm = new RollingRandomAccessFileManager(data.getLoggerContext(), raf, name, data.pattern, NullOutputStream.getInstance(), data.append, data.immediateFlush, data.bufferSize, size, time, data.policy, data.strategy, data.advertiseURI, data.layout, data.filePermissions, data.fileOwner, data.fileGroup, writeHeader); + if (rrm.isPosixSupported()) { + rrm.definePathAttributeView(file.toPath()); + } + return rrm; } catch (final IOException ex) { LOGGER.error("Cannot access RandomAccessFile " + ex, ex); if (raf != null) { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java new file mode 100644 index 0000000..7603e75 --- /dev/null +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java @@ -0,0 +1,173 @@ +/* + * 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.core.appender.rolling.action; + +import java.io.IOException; +import java.nio.file.FileVisitResult; +import java.nio.file.FileVisitor; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileOwnerAttributeView; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.List; +import java.util.Set; + +import org.apache.logging.log4j.core.Core; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.plugins.Plugin; +import org.apache.logging.log4j.core.config.plugins.PluginAttribute; +import org.apache.logging.log4j.core.config.plugins.PluginConfiguration; +import org.apache.logging.log4j.core.config.plugins.PluginElement; +import org.apache.logging.log4j.core.config.plugins.PluginFactory; +import org.apache.logging.log4j.core.lookup.StrSubstitutor; +import org.apache.logging.log4j.core.util.FileUtils; + +/** + * File posix attribute action. + */ +@Plugin(name = "PosixViewAttribute", category = Core.CATEGORY_NAME, printObject = true) +public class PosixViewAttributeAction extends AbstractPathAction { + + /** + * File permissions. + */ + private final Set filePermissions; + + /** + * File owner. + */ + private final String fileOwner; + + /** + * File group. + */ + private final String fileGroup; + + /** + * Constructor of the posix view attribute action. + * + * @param basePath {@link AbstractPathAction#getBasePath()} + * @param followSymbolicLinks {@link AbstractPathAction#isFollowSymbolicLinks()} + * @param pathConditions {@link AbstractPathAction#getPathConditions()} + * @param subst {@link AbstractPathAction#getStrSubstitutor()} + * @param filePermissions Permissions to apply + * @param fileOwner File owner + * @param fileGroup File group + * @param followSymbolicLinks + */ + public PosixViewAttributeAction(final String basePath, final boolean followSymbolicLinks, + final int maxDepth, final PathCondition[] pathConditions, final StrSubstitutor subst, + final Set filePermissions, + final String fileOwner, final String fileGroup) { + super(basePath, followSymbolicLinks, maxDepth, pathConditions, subst); + this.filePermissions = filePermissions; + this.fileOwner = fileOwner; + this.fileGroup = fileGroup; + } + + /** + * Creates a PosixViewAttributeAction action that defined posix attribute view on a file. + * + * @param basePath {@link AbstractPathAction#getBasePath()} + * @param followSymbolicLinks {@link AbstractPathAction#isFollowSymbolicLinks()} + * @param pathConditions {@link AbstractPathAction#getPathConditions()} + * @param subst {@link AbstractPathAction#getStrSubstitutor()} + * @param filePermissions File permissions + * @param fileOwner File owner + * @param fileGroup File group + * @return PosixViewAttribute action + */ + @PluginFactory + public static PosixViewAttributeAction createNameCondition( + // @formatter:off + @PluginAttribute("basePath") final String basePath, // + @PluginAttribute(value = "followLinks") final boolean followLinks, + @PluginAttribute(value = "maxDepth", defaultInt = 1) final int maxDepth, + @PluginElement("PathConditions") final PathCondition[] pathConditions, + @PluginAttribute("filePermissions") final String filePermissions, + @PluginAttribute("fileOwner") final String fileOwner, + @PluginAttribute("fileGroup") final String fileGroup, + @PluginConfiguration final Configuration config) { + // @formatter:on + return new PosixViewAttributeAction(basePath, followLinks, maxDepth, + pathConditions, config.getStrSubstitutor(), + filePermissions != null ? PosixFilePermissions.fromString(filePermissions) : null, + fileOwner, + fileGroup); + } + + @Override + protected FileVisitor createFileVisitor(final Path basePath, + final List conditions) { + return new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException { + for (final PathCondition pathFilter : conditions) { + final Path relative = basePath.relativize(file); + if (!pathFilter.accept(basePath, relative, attrs)) { + LOGGER.trace("Not defining posix attribute base={}, relative={}", basePath, relative); + return FileVisitResult.CONTINUE; + } + } + FileUtils.defineFilePosixAttributeView(file, filePermissions, fileOwner, fileGroup); + return FileVisitResult.CONTINUE; + } + }; + } + + /** + * Returns posix file permissions if defined and the OS supports posix file attribute, + * null otherwise. + * @return File posix permissions + * @see PosixFileAttributeView + */ + public Set getFilePermissions() { + return filePermissions; + } + + /** + * Returns file owner if defined and the OS supports owner file attribute view, + * null otherwise. + * @return File owner + * @see FileOwnerAttributeView + */ + public String getFileOwner() { + return fileOwner; + } + + /** + * Returns file group if defined and the OS supports posix/group file attribute view, + * null otherwise. + * @return File group + * @see PosixFileAttributeView + */ + public String getFileGroup() { + return fileGroup; + } + + @Override + public String toString() { + return "PosixViewAttributeAction [filePermissions=" + filePermissions + ", fileOwner=" + + fileOwner + ", fileGroup=" + fileGroup + ", getBasePath()=" + getBasePath() + + ", getMaxDepth()=" + getMaxDepth() + ", getPathConditions()=" + + getPathConditions() + "]"; + } + +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java index 2baa278..f82e738 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java @@ -24,7 +24,16 @@ import java.net.URI; import java.net.URL; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.UserPrincipal; +import java.nio.file.attribute.UserPrincipalLookupService; import java.util.Objects; +import java.util.Set; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.status.StatusLogger; @@ -137,4 +146,44 @@ public final class FileUtils { } } + /** + * Define file posix attribute view on a path/file. + * + * @param path Target path + * @param filePermissions Permissions to apply + * @param fileOwner File owner + * @param fileGroup File group + * @throws IOException If IO error during definition of file attribute view + */ + public static void defineFilePosixAttributeView(final Path path, + final Set filePermissions, + final String fileOwner, + final String fileGroup) throws IOException { + final PosixFileAttributeView view = Files.getFileAttributeView(path, PosixFileAttributeView.class); + if (view != null) { + final UserPrincipalLookupService lookupService = FileSystems.getDefault() + .getUserPrincipalLookupService(); + if (fileOwner != null) { + final UserPrincipal userPrincipal = lookupService.lookupPrincipalByName(fileOwner); + if (userPrincipal != null) { + // If not sudoers member, it will throw Operation not permitted + // Only processes with an effective user ID equal to the user ID + // of the file or with appropriate privileges may change the ownership of a file. + // If _POSIX_CHOWN_RESTRICTED is in effect for path + view.setOwner(userPrincipal); + } + } + if (fileGroup != null) { + final GroupPrincipal groupPrincipal = lookupService.lookupPrincipalByGroupName(fileGroup); + if (groupPrincipal != null) { + // The current user id should be members of this group, + // if not will raise Operation not permitted + view.setGroup(groupPrincipal); + } + } + if (filePermissions != null) { + view.setPermissions(filePermissions); + } + } + } } diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FilePermissionsTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FilePermissionsTest.java index ceb09e5..40c4e7b 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FilePermissionsTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FilePermissionsTest.java @@ -18,123 +18,227 @@ package org.apache.logging.log4j.core.appender; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.charset.Charset; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.PosixFilePermissions; import java.util.Arrays; import java.util.Collection; +import java.util.List; -import org.apache.commons.lang3.SystemUtils; import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.Layout; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.impl.Log4jLogEvent; import org.apache.logging.log4j.core.layout.PatternLayout; -import org.apache.logging.log4j.junit.CleanFiles; +import org.apache.logging.log4j.junit.LoggerContextRule; import org.apache.logging.log4j.message.SimpleMessage; -import org.junit.AfterClass; import org.junit.Assert; import org.junit.Assume; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.RuleChain; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; /** * Tests {@link FileAppender}. */ -@RunWith(Parameterized.class) public class FilePermissionsTest { - @Parameterized.Parameters(name = "{0} {1}") - public static Collection data() { - return Arrays.asList(new Object[][] { // - // @formatter:off - {"rwxrwxrwx", true}, - {"rw-r--r--", false}, - {"rw-------", true}, - }); - // @formatter:on - } + private static final String DIR = "target/permissions1"; - @BeforeClass - public static void beforeClass() { - // TEMP - // TODO Fix on non-Windows. - // Assume.assumeTrue(SystemUtils.IS_OS_WINDOWS); - } + @RunWith(Parameterized.class) + public static final class TestParameterized { - private final boolean createOnDemand; - private final String filePermissions; + @Parameterized.Parameters(name = "{0} {1} {2}") + public static Collection data() throws IOException { + return Arrays.asList(new Object[][] { // + // @formatter:off + {"rwxrwxrwx", true, 2}, + {"rw-r--r--", false, 3}, + {"rw-------", true, 4}, + {"rw-rw----", false, 5}, + }); + // @formatter:on + } - public FilePermissionsTest(final String filePermissions, final boolean createOnDemand) { - this.filePermissions = filePermissions; - this.createOnDemand = createOnDemand; - } + private final boolean createOnDemand; + private final String filePermissions; + private final int fileIndex; - private static final String FILE_NAME = "target/fileAppenderTest.log"; - private static final Path PATH = Paths.get(FILE_NAME); + public TestParameterized(final String filePermissions, final boolean createOnDemand, final int fileIndex) { + this.filePermissions = filePermissions; + this.createOnDemand = createOnDemand; + this.fileIndex = fileIndex; + } - @Rule - public CleanFiles files = new CleanFiles(PATH); + @BeforeClass + public static void beforeClass() { + Assume.assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); + } - @AfterClass - public static void cleanupClass() { - assertTrue("Manager for " + FILE_NAME + " not removed", !AbstractManager.hasManager(FILE_NAME)); + @Test + public void testFilePermissionsAPI() throws Exception { + final File file = new File(DIR, "AppenderTest-" + fileIndex + ".log"); + final Path path = file.toPath(); + final Layout layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) + .build(); + // @formatter:off + final FileAppender appender = FileAppender.newBuilder() + .withFileName(file.getAbsolutePath()) + .withName("test") + .withImmediateFlush(false) + .withIgnoreExceptions(false) + .withBufferedIo(false) + .withBufferSize(1) + .withLayout(layout) + .withCreateOnDemand(createOnDemand) + .withFilePermissions(filePermissions) + .build(); + // @formatter:on + try { + appender.start(); + assertTrue("Appender did not start", appender.isStarted()); + Assert.assertNotEquals(createOnDemand, Files.exists(path)); + long curLen = file.length(); + long prevLen = curLen; + assertTrue("File length: " + curLen, curLen == 0); + for (int i = 0; i < 100; ++i) { + final LogEvent event = Log4jLogEvent.newBuilder().setLoggerName("TestLogger") // + .setLoggerFqcn(FilePermissionsTest.class.getName()).setLevel(Level.INFO) // + .setMessage(new SimpleMessage("Test")).setThreadName(this.getClass().getSimpleName()) // + .setTimeMillis(System.currentTimeMillis()).build(); + try { + appender.append(event); + curLen = file.length(); + assertTrue("File length: " + curLen, curLen > prevLen); + // Give up control long enough for another thread/process to occasionally do something. + Thread.sleep(25); + } catch (final Exception ex) { + throw ex; + } + prevLen = curLen; + } + assertEquals(filePermissions, PosixFilePermissions.toString(Files.getPosixFilePermissions(path))); + } finally { + appender.stop(); + } + assertFalse("Appender did not stop", appender.isStarted()); + } + } - @Test - public void testFilePermissions() throws Exception { - Assume.assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); - final Layout layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) + public static class TestNotParameterized { + private static final String CONFIG = "log4j-posix.xml"; + + public static LoggerContextRule loggerContextRule = LoggerContextRule.createShutdownTimeoutLoggerContextRule(CONFIG); + + @Rule + public RuleChain chain = loggerContextRule.withCleanFoldersRule(DIR); + + @BeforeClass + public static void beforeClass() { + Assume.assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); + } + + @Test + public void testFilePermissions() throws Exception { + Logger logger = loggerContextRule.getLogger(FilePermissionsTest.class); + for (int i = 0; i < 1000; ++i) { + String message = "This is test message number " + i; + logger.debug(message); + } + assertEquals("rw-------", PosixFilePermissions.toString( + Files.getPosixFilePermissions(Paths.get("target/permissions1/AppenderTest-1.log")))); + } + + @Test + public void testFileUserGroupAPI() throws Exception { + final File file = new File(DIR, "AppenderTest-0.log"); + final Path path = file.toPath(); + String user = findAUser(); + assertNotNull(user); + String group = findAGroup(user); + assertNotNull(group); + String filePermissions = "rw-rw-rw-"; + + final Layout layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) + .build(); + // @formatter:off + final FileAppender appender = FileAppender.newBuilder() + .withFileName(file.getAbsolutePath()) + .withName("test") + .withImmediateFlush(true) + .withIgnoreExceptions(false) + .withBufferedIo(false) + .withBufferSize(1) + .withLayout(layout) + .withFilePermissions(filePermissions) + .withFileOwner(user) + .withFileGroup(group) .build(); - // @formatter:off - final FileAppender appender = FileAppender.newBuilder() - .withFileName(FILE_NAME) - .withName("test") - .withImmediateFlush(false) - .withIgnoreExceptions(false) - .withBufferedIo(false) - .withBufferSize(1) - .withLayout(layout) - .withCreateOnDemand(createOnDemand) - .withFilePermissions(filePermissions) - .build(); - // @formatter:on - try { - appender.start(); - final File file = new File(FILE_NAME); - assertTrue("Appender did not start", appender.isStarted()); - Assert.assertNotEquals(createOnDemand, Files.exists(PATH)); - long curLen = file.length(); - long prevLen = curLen; - assertTrue("File length: " + curLen, curLen == 0); - for (int i = 0; i < 100; ++i) { - final LogEvent event = Log4jLogEvent.newBuilder().setLoggerName("TestLogger") // - .setLoggerFqcn(FilePermissionsTest.class.getName()).setLevel(Level.INFO) // - .setMessage(new SimpleMessage("Test")).setThreadName(this.getClass().getSimpleName()) // - .setTimeMillis(System.currentTimeMillis()).build(); - try { - appender.append(event); - curLen = file.length(); - assertTrue("File length: " + curLen, curLen > prevLen); - // Give up control long enough for another thread/process to occasionally do something. - Thread.sleep(25); - } catch (final Exception ex) { - throw ex; + // @formatter:on + try { + appender.start(); + assertTrue("Appender did not start", appender.isStarted()); + long curLen = file.length(); + long prevLen = curLen; + assertTrue("File length: " + curLen, curLen == 0); + for (int i = 0; i < 100; ++i) { + final LogEvent event = Log4jLogEvent.newBuilder().setLoggerName("TestLogger") // + .setLoggerFqcn(FilePermissionsTest.class.getName()).setLevel(Level.INFO) // + .setMessage(new SimpleMessage("Test")).setThreadName(this.getClass().getSimpleName()) // + .setTimeMillis(System.currentTimeMillis()).build(); + try { + appender.append(event); + curLen = file.length(); + assertTrue("File length: " + curLen, curLen > prevLen); + // Give up control long enough for another thread/process to occasionally do something. + Thread.sleep(25); + } catch (final Exception ex) { + throw ex; + } + prevLen = curLen; } - prevLen = curLen; + assertEquals(filePermissions, PosixFilePermissions.toString(Files.getPosixFilePermissions(path))); + assertEquals(user, Files.getOwner(path).getName()); + assertEquals(group, Files.readAttributes(path, PosixFileAttributes.class).group().getName()); + } finally { + appender.stop(); } - assertEquals(filePermissions, PosixFilePermissions.toString(Files.getPosixFilePermissions(PATH))); - } finally { - appender.stop(); + assertFalse("Appender did not stop", appender.isStarted()); + } + + public static String findAGroup(String user) throws IOException { + String group = null; + try (FileInputStream fis = new FileInputStream("/etc/group")) { + List groups = org.apache.commons.io.IOUtils.readLines(fis, Charset.defaultCharset()); + for (int i = 0; i < groups.size(); i++) { + String aGroup = groups.get(i); + if (!aGroup.startsWith(user) && aGroup.contains(user)) { + group = aGroup.split(":")[0]; + break; + } + } + } + return group; + } + + private static String findAUser() throws IOException { + // On jenkins build within ubuntu, it is not possible to chmod to another user + return System.getProperty("user.name"); } - assertFalse("Appender did not stop", appender.isStarted()); } } diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderSizeCompressPermissionsTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderSizeCompressPermissionsTest.java new file mode 100644 index 0000000..f6c5b1a --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderSizeCompressPermissionsTest.java @@ -0,0 +1,103 @@ +/* + * 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.core.appender.rolling; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.concurrent.TimeUnit; + +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.junit.LoggerContextRule; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.RuleChain; + +/** + * LOG4J2-1699. + */ +public class RollingAppenderSizeCompressPermissionsTest { + + private static final String CONFIG = "log4j-rolling-gz-posix.xml"; + + private static final String DIR = "target/rollingpermissions1"; + + public static LoggerContextRule loggerContextRule = LoggerContextRule + .createShutdownTimeoutLoggerContextRule(CONFIG); + + @Rule + public RuleChain chain = loggerContextRule.withCleanFoldersRule(DIR); + + private Logger logger; + + @Before + public void setUp() throws Exception { + this.logger = loggerContextRule.getLogger(RollingAppenderSizeCompressPermissionsTest.class + .getName()); + } + + @Test + public void testAppenderCompressPermissions() throws Exception { + Assume.assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); + for (int i = 0; i < 500; ++i) { + String message = "This is test message number " + i; + logger.debug(message); + if (i % 100 == 0) { + Thread.sleep(500); + } + } + if (!loggerContextRule.getLoggerContext().stop(30, TimeUnit.SECONDS)) { + System.err.println("Could not stop cleanly " + loggerContextRule + " for " + this); + } + final File dir = new File(DIR); + assertTrue("Directory not created", dir.exists()); + final File[] files = dir.listFiles(); + assertNotNull(files); + int gzippedFiles1 = 0; + int gzippedFiles2 = 0; + for (File file : files) { + FileExtension ext = FileExtension.lookupForFile(file.getName()); + if (ext != null) { + if (file.getName().startsWith("test1")) { + gzippedFiles1++; + assertEquals("rw-------", PosixFilePermissions.toString(Files + .getPosixFilePermissions(file.toPath()))); + } else { + gzippedFiles2++; + assertEquals("r--r--r--", PosixFilePermissions.toString(Files + .getPosixFilePermissions(file.toPath()))); + } + } else { + if (file.getName().startsWith("test1")) { + assertEquals("rw-------", PosixFilePermissions.toString(Files + .getPosixFilePermissions(file.toPath()))); + } else { + assertEquals("rwx------", PosixFilePermissions.toString(Files + .getPosixFilePermissions(file.toPath()))); + } + } + } + assertTrue("Files not rolled : " + files.length, files.length > 2); + assertTrue("Files 1 gzipped not rolled : " + gzippedFiles1, gzippedFiles1 > 0); + assertTrue("Files 2 gzipped not rolled : " + gzippedFiles2, gzippedFiles2 > 0); + } +} diff --git a/log4j-core/src/test/resources/log4j-posix.xml b/log4j-core/src/test/resources/log4j-posix.xml new file mode 100644 index 0000000..48a2c88 --- /dev/null +++ b/log4j-core/src/test/resources/log4j-posix.xml @@ -0,0 +1,14 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/log4j-core/src/test/resources/log4j-rolling-gz-posix.xml b/log4j-core/src/test/resources/log4j-rolling-gz-posix.xml new file mode 100644 index 0000000..eb95d90 --- /dev/null +++ b/log4j-core/src/test/resources/log4j-rolling-gz-posix.xml @@ -0,0 +1,70 @@ + + + + + + + + + + + + %m%n + + + + + + %m%n + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file -- 1.9.5.msysgit.1