# HG changeset patch
# User Jon VanAlten <vanaltj@gmail.com>
# Date 1400807283 21600
# Node ID 9a8d79ed284488772a30c4bdcb16e5e66bf1ce54
# Parent  8f05f8a277d9f5d88e4030a91a555dce3321e9ff
Fix problem with new line in agent.auth file

Fix AgentStorageCredentials to prevent crashing bug when agent.auth
file contains an empty line.
PR1788

Reviewed-by: jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2014-March/009444.html

diff -r 8f05f8a277d9 -r 9a8d79ed2844 agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStorageCredentials.java
--- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStorageCredentials.java	Wed May 21 18:45:24 2014 +0200
+++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStorageCredentials.java	Thu May 22 19:08:03 2014 -0600
@@ -52,15 +52,21 @@
 
     private static final Translate<LocaleResources> t = LocaleResources.createLocalizer();
 
-    private static char[] pw = {'p', 'a', 's', 's', 'w', 'o', 'r', 'd'};
-    private static char[] user = {'u', 's', 'e', 'r', 'n', 'a', 'm', 'e'};
+    private static final char[] pw = {'p', 'a', 's', 's', 'w', 'o', 'r', 'd'};
+    private static final char[] user = {'u', 's', 'e', 'r', 'n', 'a', 'm', 'e'};
     private static String newLine = System.lineSeparator();
-    private static char comment = '#';
+    private static final char comment = '#';
 
-    final File authFile;
-    private String username = null; // default value
+    private final File authFile;
+    private final Reader testingAuthReader;
+    private final int authDataLength;
+    private String username;
 
     public AgentStorageCredentials(File agentAuthFile) {
+        if (agentAuthFile == null) {
+            throw new IllegalArgumentException("agentAuthFile must not be null");
+        }
+        this.testingAuthReader = null;
         this.authFile = agentAuthFile;
         long length = authFile.length();
         if (length > Integer.MAX_VALUE || length < 0L) {
@@ -73,9 +79,31 @@
             }
             throw new InvalidConfigurationException(t.localize(LocaleResources.FILE_NOT_VALID, authPath));
         }
+        authDataLength = (int) length;
         // Cache username but not password, instead read that on demand to prevent heap dump
         // password leak attack.
-        initUsernameFromFile();
+        initUsername();
+    }
+
+    // Testing constructor
+    AgentStorageCredentials(Reader agentAuthReader) {
+        if (agentAuthReader == null) {
+            throw new IllegalArgumentException("agentAuthReader must not be null");
+        }
+        this.testingAuthReader = agentAuthReader;
+        long length = -1;
+        try {
+            length = testingAuthReader.skip(Long.MAX_VALUE);
+            if (length > Integer.MAX_VALUE) {
+                throw new IllegalArgumentException("agentAuthReader larger than supported Integer.MAX_VALUE");
+            }
+            testingAuthReader.reset();
+        } catch (IOException e) {
+            throw new IllegalArgumentException("IOException from agentAuthReader", e);
+        }
+        authDataLength = (int) length;
+        this.authFile = null;
+        initUsername();
     }
 
     @Override
@@ -85,51 +113,76 @@
 
     @Override
     public char[] getPassword() {
-        return readPasswordFromFile();
+        return readPassword();
     }
 
-    private void initUsernameFromFile() {
-        char[] authData = getAuthFileData();
+    private Reader getReader() throws IOException {
+        if (testingAuthReader != null) {
+           return testingAuthReader;
+        }
+        if (authFile == null || !authFile.canRead() || !authFile.isFile()) {
+            throw new IllegalStateException("Invalid agent.auth file: " + authFile.getCanonicalPath());
+        }
+        return new InputStreamReader(new FileInputStream(authFile), StandardCharsets.US_ASCII);
+    }
+
+    private void initUsername() {
+        char[] authData = getAuthData();
         if (authData == null) {
             return;
         }
         try {
-            setUsernameFromData(authData, (int) authFile.length());
+            setUsernameFromData(authData, authDataLength);
         } finally {
             clearCharArray(authData);
         }
     }
 
-    private char[] readPasswordFromFile() {
-        char[] authData = getAuthFileData();
+    private char[] readPassword() {
+        char[] authData = getAuthData();
         if (authData == null) {
             return null;
         }
         char[] password = null;
         try {
-            password = getValueFromData(authData, (int) authFile.length(), pw);
+            password = getValueFromData(authData, authDataLength, pw);
         } finally {
             clearCharArray(authData);
         }
         return password;
     }
 
-    private char[] getAuthFileData() {
+    private char[] getAuthData() {
         char[] authData = null;
-        if (authFile != null && authFile.canRead() && authFile.isFile()) {
-            int length = (int) authFile.length(); // Verified during constructor that this cast is safe
-            try (Reader reader = new InputStreamReader(new FileInputStream(authFile), StandardCharsets.US_ASCII)) {
-                // file size in bytes >= # of chars so this size should be sufficient.
-                authData = new char[length];
-                // This is probably the most sensitive time for password-in-heap exposure.
-                // The reader here may contain buffers containing the password.  It will,
-                // of course, be garbage collected in due time.
-                int chars = reader.read(authData, 0, length);
-                if (chars != length) {
-                    throw new InvalidConfigurationException("End of auth file stream reached unexpectedly.");
+        Reader reader = null;
+        try {
+            try {
+                reader = getReader();
+            } catch (IllegalStateException e) {
+                // Callers will assume null auth parameters.
+                return null;
+            }
+            // file size in bytes >= # of chars so this size should be sufficient.
+            authData = new char[authDataLength];
+            // This is probably the most sensitive time for password-in-heap exposure.
+            // The reader here may contain buffers containing the password.  It will,
+            // of course, be garbage collected in due time.
+            int chars = reader.read(authData, 0, authDataLength);
+            if (chars != authDataLength) {
+                throw new InvalidConfigurationException("End of auth file stream reached unexpectedly.");
+            }
+            if (reader == testingAuthReader) {
+                reader.reset();
+            }
+        } catch (IOException e) {
+            throw new InvalidConfigurationException(e);
+        } finally {
+            if (reader != testingAuthReader) {
+                try {
+                    reader.close();
+                } catch (IOException e) {
+                    e.printStackTrace();
                 }
-            } catch (IOException e) {
-                throw new InvalidConfigurationException(e);
             }
         }
         return authData;
@@ -149,7 +202,7 @@
     private char[] getValueFromData(char[] data, int dataLen, char[] target) {
         int position = 0;
         while (position < dataLen) {
-            if ((position + 1 == dataLen) || data[position + 1] == newLine.charAt(0)) {
+            if ((position + 1 == dataLen) || data[position] == newLine.charAt(0)) {
                 // Empty line
                 position = nextLine(data, position);
                 continue;
@@ -190,7 +243,7 @@
     }
 
     private int nextLine(char[] data, int current) {
-        int next = current + 1;
+        int next = current;
         while (next < data.length) {
             if (data[next] == newLine.charAt(0)) {
                 break;
diff -r 8f05f8a277d9 -r 9a8d79ed2844 agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentStorageCredentialsTest.java
--- a/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentStorageCredentialsTest.java	Wed May 21 18:45:24 2014 +0200
+++ b/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentStorageCredentialsTest.java	Thu May 22 19:08:03 2014 -0600
@@ -36,57 +36,51 @@
 package com.redhat.thermostat.agent.config;
 
 import java.io.File;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.util.Random;
+import java.io.Reader;
+import java.io.StringReader;
 
 import org.junit.Assert;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 public class AgentStorageCredentialsTest {
 
-    private static Random random;
-
-    @BeforeClass
-    public static void setUpOnce() {
-        random = new Random();
-    }
-
     @Test
-    public void testAuthConfigFromFile() throws IOException {
-        File tmpAuth = createTempAuthFile("username=user\npassword=pass\n");
-        AgentStorageCredentials creds = new AgentStorageCredentials(tmpAuth);
+    public void testAuthConfig() {
+        Reader reader = new StringReader("username=user\npassword=pass\n");
+        AgentStorageCredentials creds = new AgentStorageCredentials(reader);
         Assert.assertEquals("user", creds.getUsername());
         Assert.assertEquals("pass", new String(creds.getPassword()));
     }
 
     @Test
-    public void testAuthConfigFromEmptyFile() throws IOException {
-        File tmpAuth = createTempAuthFile("");
-        AgentStorageCredentials creds = new AgentStorageCredentials(tmpAuth);
+    public void testEmptyAuthConfig() {
+        Reader reader = new StringReader("");
+        AgentStorageCredentials creds = new AgentStorageCredentials(reader);
+        Assert.assertNull(creds.getUsername());
+        Assert.assertNull(creds.getPassword());
+    }
+
+    @Test
+    public void testNonExistingAgentAuthFile() {
+        File file = new File("this.file.should.not.exist");
+        AgentStorageCredentials creds = new AgentStorageCredentials(file);
         Assert.assertNull(creds.getUsername());
         Assert.assertNull(creds.getPassword());
     }
 
     @Test
-    public void testAuthConfigWithConfigCommentedOut() throws IOException {
-        File tmpAuth = createTempAuthFile("#username=user\n#password=pass\n");
-        AgentStorageCredentials creds = new AgentStorageCredentials(tmpAuth);
+    public void testAuthConfigWithConfigCommentedOut() {
+        Reader reader = new StringReader("#username=user\n#password=pass\n");
+        AgentStorageCredentials creds = new AgentStorageCredentials(reader);
         Assert.assertNull(creds.getUsername());
         Assert.assertNull(creds.getPassword());
     }
 
-    private File createTempAuthFile(String contents) throws IOException {
-        String tmpAuthLoc = System.getProperty("java.io.tmpdir") + File.separatorChar +
-                Math.abs(random.nextInt());
-        File tmpAuth = new File(tmpAuthLoc);
-        tmpAuth.deleteOnExit();
-        tmpAuth.createNewFile();
-        FileWriter authWriter = new FileWriter(tmpAuth);
-        authWriter.append(contents);
-        authWriter.flush();
-        authWriter.close();
-        return tmpAuth;
+    @Test
+    public void testAuthConfigWithVariousExtraNewlines() {
+        Reader reader = new StringReader("\n#username=nottheuser\n\n\n#password=wrong\nusername=user\n\n\npassword=pass\n\n\n#username=wronguser\n\n\n#password=badpassword");
+        AgentStorageCredentials creds = new AgentStorageCredentials(reader);
+        Assert.assertEquals("user", creds.getUsername());
+        Assert.assertEquals("pass", new String(creds.getPassword()));
     }
 }