honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove print writer wrapping for users tools 

When writing the users and users_roles files, we wrap a custom writer in
a print writer. There is a problem with this though: when print writer
closes it closes our underlying custom writer and the close
implementation for our custom writer is not trivial, it executes code
that can throw an I/O exception. When print writer invokes this close
and an I/O exception is thrown, it swallows that exception and sets the
status on the print writer to error. One would think that we could
simply check this status but alas print writer is broken here. The act
of checking the status causes print writer to try to flush the
underyling stream which is going to be completely undefined because the
underlying stream might or might not be closed. This might cause another
exception to be thrown, losing the original. Print writer screwed the
pooch here, there is no good reason to try to do any I/O after the
underlying writer entered a failed state. To address this we remove the
use of print writer, we use our custom writer directly. This allows any
thrown exceptions to bubble up.

Relates elastic/x-pack-elasticsearch#2288

Original commit: elastic/x-pack-elasticsearch@11b8dd5641
This commit is contained in:
Jason Tedor 2017-08-16 12:50:39 -04:00 committed by GitHub
parent 6fcc3be438
commit 2bf8f4b0bc
3 changed files with 16 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
plugin/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserPasswdStore.java
View File

@ -27,7 +27,7 @@ import org.elasticsearch.xpack.security.support.Validation.Users;
import org.elasticsearch.xpack.security.user.User;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.Writer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
@ -169,9 +169,11 @@ public class FileUserPasswdStore {
}
public static void writeFile(Map<String, char[]> users, Path path) {
try (PrintWriter writer = new PrintWriter(openAtomicMoveWriter(path))) {
try (Writer writer = openAtomicMoveWriter(path)) {
for (Map.Entry<String, char[]> entry : users.entrySet()) {
writer.printf(Locale.ROOT, "%s:%s%s", entry.getKey(), new String(entry.getValue()), System.lineSeparator());
final String message =
String.format(Locale.ROOT, "%s:%s%s", entry.getKey(), new String(entry.getValue()), System.lineSeparator());
writer.write(message);
}
} catch (IOException ioe) {
throw new ElasticsearchException("could not write file [{}], please check file permissions", ioe, path.toAbsolutePath());

12
plugin/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStore.java
View File

@ -21,7 +21,7 @@ import org.elasticsearch.xpack.security.support.NoOpLogger;
import org.elasticsearch.xpack.security.support.Validation;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.Writer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
@ -193,10 +193,14 @@ public class FileUserRolesStore {
}
}
try (PrintWriter writer = new PrintWriter(openAtomicMoveWriter(path))) {
try (Writer writer = openAtomicMoveWriter(path)) {
for (Map.Entry<String, List<String>> entry : roleToUsers.entrySet()) {
writer.printf(Locale.ROOT, "%s:%s%s", entry.getKey(), Strings.collectionToCommaDelimitedString(entry.getValue()),
System.lineSeparator());
final String message = String.format(
Locale.ROOT,
"%s:%s%s",
entry.getKey(),
Strings.collectionToCommaDelimitedString(entry.getValue()), System.lineSeparator());
writer.write(message);
}
} catch (IOException ioe) {
throw new ElasticsearchException("could not write file [" + path.toAbsolutePath() + "], please check file permissions", ioe);

7
plugin/src/test/java/org/elasticsearch/xpack/security/support/SecurityFilesTests.java
View File

@ -11,14 +11,13 @@ import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
import java.io.PrintWriter;
import java.io.Writer;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFilePermission;
import java.util.Locale;
import java.util.Set;
import static java.nio.file.attribute.PosixFilePermission.GROUP_EXECUTE;
@ -49,8 +48,8 @@ public class SecurityFilesTests extends ESTestCase {
Files.setPosixFilePermissions(path, perms);
try (PrintWriter writer = new PrintWriter(openAtomicMoveWriter(path))) {
writer.printf(Locale.ROOT, "This is a test");
try (Writer writer = openAtomicMoveWriter(path)) {
writer.write("This is a test");
}
Set<PosixFilePermission> permissionsAfterWrite = Files.getPosixFilePermissions(path);