Add doPrivilege blocks for socket connect ops in repository-hdfs#22793
Add doPrivilege blocks for socket connect ops in repository-hdfs#22793Tim-Brooks merged 8 commits intoelastic:masterfrom
Conversation
|
In hdfs we are calling |
We do this to limit what craziness hdfs can do (and know exactly what it requires). |
s1monw
left a comment
There was a problem hiding this comment.
I left one comment looks great so far
| try { | ||
| SpecialPermission.check(); | ||
| // FSDataInputStream can open connection on read() | ||
| return AccessController.doPrivileged((PrivilegedExceptionAction<Integer>) is::read); |
There was a problem hiding this comment.
can we maybe try to trigger this differently? I mean can we for instance try to call #available() or can we maybe read the first byte on open and wrap in a BufferedInputStream and then do this:
InputStream stream = is.isMarkSupported() ? is : new BufferedInputStream(is);
// do the following in doPrivileged?
stream.mark(1);
stream.skip(1);
stream.reset();
return stream;|
@s1monw In my testing available() was not doing the trick. So I went with the buffered, mark, skip, reset approach. |
s1monw
left a comment
There was a problem hiding this comment.
left one more comment we are close I think
| } | ||
| } | ||
| }; | ||
| InputStream stream = is.markSupported() ? is : new BufferedInputStream(is); |
There was a problem hiding this comment.
if we run into an exception here we have to close the stream.
we usually do this:
boolean success = false;
try {
// do something with the stream
success = true;
return stream;
} finally {
if (success == false) {
IOUtils.closeWhileHandlingException(stream);
}
}|
Note to other reviews: Simon and I talked about this for a while and decided it was safest to return a |
…-hdfs (elastic#22793)" Only pulled the relevant changes - such as the Priveleged input stream implementation for HDFS.
This is related to #22116. The repository-hdfs plugin opens socket
connections. As
SocketPermissionis transitioned out of core, hdfswill require
connectpermission. This pull request wraps operationsthat require this permission in
doPrivilegedblocks.