[
https://issues.apache.org/jira/browse/GEODE-8572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17209219#comment-17209219
]
ASF GitHub Bot commented on GEODE-8572:
---------------------------------------
demery-pivotal commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500649130
##########
File path:
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws
IOException {
}
private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector)
throws IOException {
- Stream<Path> selectedFiles/* = null */;
if (!workingDir.toFile().isDirectory()) {
return Collections.emptyList();
}
- selectedFiles =
Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
- return selectedFiles.collect(toList());
+ return Files.list(workingDir)
+ .filter(Files::isRegularFile)
Review comment:
Yes, that's what I thought, and I verified with some experiments (which
did not involve the exporter).
My question remains: If the path we're testing is a symbolic link, are you
saying that you _want_ to ignore the file it links to? If so, what makes you
want to skip the file in that case?
One possibility: The link links to a file in the same dir, which would lead
us to export the content twice (once via the file, and once via the link to the
file). We could solve this by resolving the real path to the file, and
collecting the matching files into a Set instead of a List.
Another possibility: The link links to a file in some other directory, and
some concern makes you not want to read the file from the other directory.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> LogExporter throws if a directory matches its file selector
> -----------------------------------------------------------
>
> Key: GEODE-8572
> URL: https://issues.apache.org/jira/browse/GEODE-8572
> Project: Geode
> Issue Type: Bug
> Components: gfsh
> Affects Versions: 1.13.0
> Reporter: Dale Emery
> Priority: Major
> Labels: GeodeOperationAPI, pull-request-available
>
> {{LogExporter}} tries to read and export any directory entry whose name is
> accepted by its log file selector predicate. The predicate accepts any entry
> whose name, when converted to lower case, contains ".log". If one of those
> entries is directory, the predicate accepts it anyway. When {{LogExporter}}
> attempts to create a {{FileReader}} read it, the {{FileReader}} constructor
> throws {{FileNotFoundException}}.
> There is a stat file selector predicate that works similarly.
> {{LogExporter}}'s log and stat file selector predicates should accept only
> files, and not directories. And perhaps the should accept only files whose
> names _end_ in the appropriate extension, rather than _containing_ the
> characters. The predicates are defined in {{LogExporter}}'s
> {{findLogFiles()}} and {{findStatFiles()}} methods.
> I discovered this defect by running {{LogExporterIntegrationTest}} on macOS.
> Each test's preparation writes some to-be-exported files into a temporary
> directory that, on macOS, may contain numerous other files and directories.
> One of those directories (e.g.
> /var/folders/yz/6y59fxln38d7lf2jxng1zgg40000gn/T/com.apple.LoginUserService),
> which matches the {{LogExporter}}'s predicate.
> These tests should also be changed to write their to-be-exported files to a
> directory that is known to be empty.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
