Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(817)

Issue 39390046: FSA Do not crawl hidden files or directories. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 3 months ago by Brett
Modified:
10 years, 3 months ago
Reviewers:
ejona, mifern, pjo
CC:
connector-cr_google.com
Visibility:
Public.

Description

This change avoids crawling hidden files or directories. When crawling a directory, hidden files and subdirectories are not included in the returned content listing. This change also verifies that the requested file/folder does not reside within a hidded ancester directory. One exception is made. Windows Administrative Shares are hidden by default, so we allow the root start point to be hidden, but not its descendents. This is consistent with the behaviour of the filesystem connector. This change includes another minor enhancement. It verifies that the traversal user has read access to a file or folder before returning it.

Patch Set 1 #

Total comments: 8

Patch Set 2 : Drop check for isReadable. #

Total comments: 2

Patch Set 3 : Drop check for administrative shares. Better logging for hidden items. #

Total comments: 3

Patch Set 4 : OK. Feed hidden directories and files, but return not found on crawl request. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -9 lines) Patch
M src/com/google/enterprise/adaptor/fs/FileDelegate.java View 1 1 chunk +6 lines, -0 lines 0 comments Download
M src/com/google/enterprise/adaptor/fs/FsAdaptor.java View 1 2 3 2 chunks +21 lines, -9 lines 0 comments Download
M src/com/google/enterprise/adaptor/fs/NioFileDelegate.java View 1 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 21
Brett
10 years, 3 months ago (2013-12-12 02:36:10 UTC) #1
ejona
I'm not sure I like the isReadable check. It seems like it actually makes it ...
10 years, 3 months ago (2013-12-12 17:20:42 UTC) #2
ejona
I'm not sure I like the isReadable check. It seems like it actually makes it ...
10 years, 3 months ago (2013-12-12 17:20:43 UTC) #3
Brett
https://codereview.appspot.com/39390046/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/39390046/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode296 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:296: log.log(Level.WARNING, "The path {0} is not a supported file ...
10 years, 3 months ago (2013-12-12 17:37:12 UTC) #4
ejona
https://codereview.appspot.com/39390046/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/39390046/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode428 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:428: && delegate.isReadable(p) && !isHidden(p); On 2013/12/12 17:37:12, Brett wrote: ...
10 years, 3 months ago (2013-12-12 17:47:05 UTC) #5
brett.michael.johnson_gmail.com
The File System Connector customers did not like exceptions being thrown when the traversal user ...
10 years, 3 months ago (2013-12-12 17:51:52 UTC) #6
mifern
I agree with Eric the isReadable check is not needed. I would prefer an exception ...
10 years, 3 months ago (2013-12-12 17:55:21 UTC) #7
Brett
Drop check for isReadable.
10 years, 3 months ago (2013-12-13 01:27:19 UTC) #8
ejona
LGTM
10 years, 3 months ago (2013-12-13 01:41:32 UTC) #9
mifern
https://codereview.appspot.com/39390046/diff/20001/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/39390046/diff/20001/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode286 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:286: if (!isVisibleDescendantOfRoot(doc)) { I think we should make a ...
10 years, 3 months ago (2013-12-13 01:56:20 UTC) #10
brett.michael.johnson_gmail.com
-- Brett M. Johnson On Dec 12, 2013, at 5:56 PM, mifern@google.com wrote: > > ...
10 years, 3 months ago (2013-12-13 18:43:00 UTC) #11
mifern
The statement about admin shares being hidden is not true. I can't comment on SmbFile.isHidden, ...
10 years, 3 months ago (2013-12-13 18:57:34 UTC) #12
Brett
Drop check for administrative shares. Better logging for hidden items.
10 years, 3 months ago (2013-12-13 19:36:47 UTC) #13
mifern
https://codereview.appspot.com/39390046/diff/40001/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/39390046/diff/40001/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode425 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:425: && !delegate.isHidden(p); Is calling delegate.isHidden really needed here? Hidden ...
10 years, 3 months ago (2013-12-13 22:16:04 UTC) #14
ejona
https://codereview.appspot.com/39390046/diff/40001/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/39390046/diff/40001/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode425 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:425: && !delegate.isHidden(p); On 2013/12/13 22:16:05, mifern wrote: > Is ...
10 years, 3 months ago (2013-12-13 22:54:39 UTC) #15
Brett
https://codereview.appspot.com/39390046/diff/40001/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/39390046/diff/40001/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode425 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:425: && !delegate.isHidden(p); On 2013/12/13 22:16:05, mifern wrote: > Is ...
10 years, 3 months ago (2013-12-13 23:42:49 UTC) #16
Brett
OK. Feed hidden directories and files, but return not found on crawl request.
10 years, 3 months ago (2013-12-14 00:40:27 UTC) #17
ejona
On 2013/12/14 00:40:27, Brett wrote: > OK. Feed hidden directories and files, but return not ...
10 years, 3 months ago (2013-12-14 00:50:19 UTC) #18
brett.michael.johnson_gmail.com
If it was up to me, I wouldn’t feed items that were hidden or unreadable. ...
10 years, 3 months ago (2013-12-14 01:26:42 UTC) #19
mifern
LGTM
10 years, 3 months ago (2013-12-18 15:50:43 UTC) #20
Brett
10 years, 3 months ago (2013-12-18 19:06:33 UTC) #21
Committed 13 December 2013 to File System Adaptor
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b