Attachment 612646 Details for Bug 839080 – fix and test for this issue

[patch] fix and test for this issue

0001-Bug-839080-ProcessInfoQuery-does-not-properly-handle.patch (text/plain), 8.50 KB, created by Elias Ross on 2012-09-13 21:57:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Elias Ross

Created: 2012-09-13 21:57:05 UTC

Size: 8.50 KB

patch

obsolete

>From 94bcf482148befafe124d6af82855ac64cabb75d Mon Sep 17 00:00:00 2001
>From: Elias Ross <elias_ross@apple.com>
>Date: Tue, 10 Jul 2012 14:19:49 -0700
>Subject: [PATCH] Bug 839080 - ProcessInfoQuery does not properly handle
> pidfile with line ending or whitespace
>
>---
> .../java/org/rhq/core/system/pquery/Attribute.java |    2 +-
> .../org/rhq/core/system/pquery/Conditional.java    |   21 ++++--------
> .../rhq/core/system/pquery/ProcessInfoQuery.java   |   30 ++++++++++++----
> .../core/system/pquery/ProcessInfoQueryTest.java   |   36 ++++++++++++++++----
> 4 files changed, 60 insertions(+), 29 deletions(-)
>
>diff --git a/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/Attribute.java b/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/Attribute.java
>index 5c06b66..b9962ea 100644
>--- a/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/Attribute.java
>+++ b/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/Attribute.java
>@@ -35,7 +35,7 @@ class Attribute {
>         name, basename, pidfile, pid
>     }
> 
>-    private String attributeValue;
>+    private final String attributeValue;
> 
>     Attribute(String attributeValue, Category category) {
>         validate(attributeValue, category);
>diff --git a/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/Conditional.java b/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/Conditional.java
>index 3867fe2..cb7a48d 100644
>--- a/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/Conditional.java
>+++ b/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/Conditional.java
>@@ -33,7 +33,7 @@ package org.rhq.core.system.pquery;
>  * @author John Mazzitelli
>  */
> class Conditional {
>-    char separator = '|';
>+    private char separator = '|';
> 
>     enum Category {
>         process, arg
>@@ -47,10 +47,10 @@ class Conditional {
>         unspecified, parent
>     }
> 
>-    private Category category;
>-    private Attribute attribute;
>-    private Operator operator;
>-    private Qualifier qualifier;
>+    private final Category category;
>+    private final Attribute attribute;
>+    private final Operator operator;
>+    private final Qualifier qualifier;
> 
>     Conditional(String conditional) {
>         char possibleSeparator = conditional.charAt(0);
>@@ -74,9 +74,6 @@ class Conditional {
>         try {
>             this.category = Category.valueOf(categoryString);
>         } catch (Exception e) {
>-        }
>-
>-        if (this.category == null) {
>             throw new IllegalArgumentException("Invalid category: " + conditional);
>         }
> 
>@@ -85,9 +82,6 @@ class Conditional {
>         try {
>             this.operator = Operator.valueOf(operatorString);
>         } catch (Exception e) {
>-        }
>-
>-        if (this.operator == null) {
>             throw new IllegalArgumentException("Invalid operator: " + conditional);
>         }
> 
>@@ -95,10 +89,7 @@ class Conditional {
>             try {
>                 this.qualifier = Qualifier.valueOf(qualifierString);
>             } catch (Exception e) {
>-            }
>-
>-            if (this.qualifier == null) {
>-                throw new IllegalArgumentException("Invalid qualifier: " + conditional);
>+                throw new IllegalArgumentException("Invalid qualifier: " + qualifierString);
>             }
>         } else {
>             this.qualifier = Qualifier.unspecified;
>diff --git a/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/ProcessInfoQuery.java b/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/ProcessInfoQuery.java
>index 82486ca..0a05df3 100644
>--- a/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/ProcessInfoQuery.java
>+++ b/modules/core/native-system/src/main/java/org/rhq/core/system/pquery/ProcessInfoQuery.java
>@@ -22,13 +22,20 @@
>   */
> package org.rhq.core.system.pquery;
> 
>+import java.io.BufferedReader;
> import java.io.FileInputStream;
>+import java.io.FileNotFoundException;
>+import java.io.IOException;
>+import java.io.InputStreamReader;
> import java.util.ArrayList;
> import java.util.HashMap;
> import java.util.HashSet;
> import java.util.List;
> import java.util.Map;
> import java.util.Set;
>+
>+import org.apache.commons.logging.Log;
>+import org.apache.commons.logging.LogFactory;
> import org.rhq.core.system.NativeSystemInfo;
> import org.rhq.core.system.ProcessInfo;
> import org.rhq.core.system.pquery.Conditional.Qualifier;
>@@ -182,10 +189,13 @@ import org.rhq.core.system.pquery.Conditional.Qualifier;
>  * @author John Mazzitelli
>  */
> public class ProcessInfoQuery {
>+
>+    private static final Log log = LogFactory.getLog(ProcessInfoQuery.class);
>+
>     /**
>      * The map of all processes keyed on their pids.
>      */
>-    private Map<Long, ProcessInfo> allProcesses;
>+    private final Map<Long, ProcessInfo> allProcesses;
> 
>     /**
>      * Constructor for {@link ProcessInfoQuery} given an collection of process information that represents the processes
>@@ -458,18 +468,24 @@ public class ProcessInfoQuery {
>         String contents;
> 
>         try {
>-            byte[] bytes = new byte[64]; // a pid file should never come close to being 64 bytes big
>             FileInputStream fis = new FileInputStream(pidfileName);
>+            BufferedReader br = new BufferedReader(new InputStreamReader(fis));
>             try {
>-                int count = fis.read(bytes);
>-                contents = new String(bytes, 0, count);
>+                contents = br.readLine();
>+                if (contents == null) {
>+                    throw new IOException("empty pid");
>+                }
>             } finally {
>                 fis.close();
>             }
>-        } catch (Exception e) {
>-            contents = "";
>+        } catch (FileNotFoundException e) {
>+            log.trace("pid not found");
>+            return "";
>+        } catch (IOException e) {
>+            log.warn("unable to read pid file " + pidfileName, e);
>+            return "";
>         }
> 
>-        return contents;
>+        return contents.trim();
>     }
> }
>\ No newline at end of file
>diff --git a/modules/core/native-system/src/test/java/org/rhq/core/system/pquery/ProcessInfoQueryTest.java b/modules/core/native-system/src/test/java/org/rhq/core/system/pquery/ProcessInfoQueryTest.java
>index bc8d77d..6a90ed6 100644
>--- a/modules/core/native-system/src/test/java/org/rhq/core/system/pquery/ProcessInfoQueryTest.java
>+++ b/modules/core/native-system/src/test/java/org/rhq/core/system/pquery/ProcessInfoQueryTest.java
>@@ -24,8 +24,11 @@ package org.rhq.core.system.pquery;
> 
> import java.io.File;
> import java.io.FileOutputStream;
>+import java.io.FileWriter;
> import java.util.ArrayList;
> import java.util.List;
>+import java.util.UUID;
>+
> import org.testng.annotations.BeforeMethod;
> import org.testng.annotations.Test;
> import org.rhq.core.system.ProcessInfo;
>@@ -262,16 +265,13 @@ public class ProcessInfoQueryTest {
> 
>     /**
>      * Test PID files.
>-     *
>-     * @throws Exception
>      */
>     public void testPIQLPidfile() throws Exception {
>         File pidfile = File.createTempFile("test", ".pid");
>         try {
>-            FileOutputStream fos = new FileOutputStream(pidfile);
>-            fos.write("3".getBytes());
>-            fos.flush();
>-            fos.close();
>+            FileWriter fw = new FileWriter(pidfile);
>+            fw.write("3");
>+            fw.close();
> 
>             results = query.query("process|pidfile|match=" + pidfile.getCanonicalPath());
>             assert results.size() == 1 : results;
>@@ -291,6 +291,30 @@ public class ProcessInfoQueryTest {
>     }
> 
>     /**
>+     * Test PID files with whitespace or empty files.
>+     */
>+    public void testPIQLPidfileWhitespace() throws Exception {
>+        File pidfile = File.createTempFile("test", ".pid");
>+        try {
>+            results = query.query("process|pidfile|match=" + pidfile.getCanonicalPath());
>+            assert results.size() == 0 : results;
>+
>+            FileWriter fw = new FileWriter(pidfile);
>+            fw.write(" 3\r\n");
>+            fw.close();
>+
>+            results = query.query("process|pidfile|match=" + pidfile.getCanonicalPath());
>+            assert results.size() == 1 : results;
>+        } finally {
>+            pidfile.delete();
>+        }
>+        results = query.query("process|pidfile|match=" + pidfile.getCanonicalPath());
>+        assert results.size() == 0 : results;
>+        results = query.query("process|pidfile|match=" + UUID.randomUUID());
>+        assert results.size() == 0 : results;
>+    }
>+
>+    /**
>      * Test regular expressions.
>      */
>     public void testPIQLProcessNameRegularExpression() {
>-- 
>1.7.9.3
>

Actions: View | Diff

Attachments on bug 839080: 612646 | 612649