jmestwa-coder commented on code in PR #4229:
URL: https://github.com/apache/logging-log4j2/pull/4229#discussion_r3853142551
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java:
##########
@@ -159,7 +164,8 @@ public static void defineFilePosixAttributeView(
final String fileOwner,
final String fileGroup)
throws IOException {
- final PosixFileAttributeView view = Files.getFileAttributeView(path,
PosixFileAttributeView.class);
+ final PosixFileAttributeView view =
Review Comment:
makes sense, I missed that the manager paths call this on the configured
fileName where following is the intended behavior. reverted to the plain lookup
and dropped the LinkOption import; the visitor guard carries the fix now.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java:
##########
@@ -146,6 +147,10 @@ public static void makeParentDirs(final File file) throws
IOException {
/**
* Define file POSIX attribute view on a path/file.
+ * <p>
+ * Symbolic links are never followed: if {@code path} is a link, the
attributes of the link itself are
+ * modified and its target is left untouched.
+ * </p>
*
* @param path Target path
Review Comment:
dropped the paragraph along with the revert.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java:
##########
@@ -369,6 +369,10 @@ protected FileVisitor<Path> createFileVisitor(final Path
basePath, final List<Pa
return new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult visitFile(final Path file, final
BasicFileAttributes attrs) throws IOException {
+ if (attrs.isSymbolicLink()) {
Review Comment:
good catch on followLinks="true", applied the suggestion.
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java:
##########
@@ -87,6 +90,28 @@ void testFileFromUriWithSpacesAndPlusCharactersInName()
throws Exception {
assertTrue(file.exists(), "file exists");
}
Review Comment:
added it to PosixViewAttributeActionTest next to the negative test, both
pass.
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java:
##########
@@ -87,6 +90,28 @@ void testFileFromUriWithSpacesAndPlusCharactersInName()
throws Exception {
assertTrue(file.exists(), "file exists");
}
+ @Test
Review Comment:
removed, along with the imports it pulled in.
##########
src/changelog/.2.x.x/fix_posix_view_attribute_symlink.xml:
##########
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xmlns="https://logging.apache.org/xml/ns";
+ xsi:schemaLocation="https://logging.apache.org/xml/ns
https://logging.apache.org/xml/ns/log4j-changelog-0.xsd";
+ type="fixed">
+ <issue id="4229" link="https://github.com/apache/logging-log4j2/pull/4229"/>
+ <description format="asciidoc">Stop `PosixViewAttribute` from applying
permissions and ownership through symbolic links</description>
Review Comment:
took the suggested wording.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
