Bug 46491 - Incorrect value for the last variable in "CSV Data Set Config" (error in processing quoted strings)

git-svn-id: https://svn.apache.org/repos/asf/jakarta/jmeter/trunk@740183 13f79535-47bb-0310-9956-ffa450edef68

Former-commit-id: bb1dc0669c

src/core/org/apache/jmeter/save/CSVSaveService.java

@@ -58,6 +58,7 @@ import org.apache.oro.text.regex.Perl5Matcher;
 /**
  * This class provides a means for saving/reading test results as CSV files.
  */
+// For unit tests, @see TestCSVSaveService
 public final class CSVSaveService {
     private static final Logger log = LoggingManager.getLoggerForClass();
 
@@ -137,7 +138,11 @@ public final class CSVSaveService {
                 lineNumber = 0;
             }
             String [] parts;
-            while ((parts = csvReadFile(dataReader, saveConfig.getDelimiter().charAt(0))).length != 0) {
+            final char delim = saveConfig.getDelimiter().charAt(0);
+            // TODO: does it matter that an empty line will terminate the loop?
+            // CSV output files should never contain empty lines, so probably not
+            // If so, then need to check whether the reader is at EOF
+            while ((parts = csvReadFile(dataReader, delim)).length != 0) {
                 lineNumber++;
                 SampleEvent event = CSVSaveService.makeResultFromDelimitedString(parts,saveConfig,lineNumber);
                 if (event != null){
@@ -208,6 +213,8 @@ public final class CSVSaveService {
                         timeStamp = Long.parseLong(text);
                     } catch (NumberFormatException e) {// see if this works
                         log.warn(e.toString());
+                        // method is only ever called from one thread at a time
+                        // so it's OK to use a static DateFormat
                         Date stamp = DEFAULT_DATE_FORMAT.parse(text);
                         timeStamp = stamp.getTime();
                         log.warn("Setting date format to: "+DEFAULT_DATE_FORMAT_STRING);
@@ -911,9 +918,11 @@ public final class CSVSaveService {
     /**
      * Reads from file and splits input into strings according to the delimiter,
      * taking note of quoted strings.
-     *
+     * <p>
      * Handles DOS (CRLF), Unix (LF), and Mac (CR) line-endings equally.
-     *
+     * <p>
+     * N.B. a blank line is returned as a zero length array, whereas "" is returned
+     * as an empty string. This is inconsistent.
      * @param infile input file  - must support mark(1)
      * @param delim delimiter (e.g. comma)
      * @return array of strings
@@ -924,8 +933,9 @@ public final class CSVSaveService {
         int state = INITIAL;
         List list = new ArrayList();
         ByteArrayOutputStream baos = new ByteArrayOutputStream(200);
-        while(-1 != (ch=infile.read())){
         boolean push = false;
+        while(-1 != (ch=infile.read())){
+            push = false;
             switch(state){
             case INITIAL:
                 if (ch == QUOTING_CHAR){
@@ -983,15 +993,19 @@ public final class CSVSaveService {
                 break;
             }
         } // while not EOF
-        if (ch == -1){
+        if (ch == -1){// EOF (or end of string) so collect any remaining data
             if (state == QUOTED){
                 throw new IOException(state+" Missing trailing quote-char in quoted field:[\""+baos.toString()+"]");
             }
-            if (baos.size() > 0) {
+            // Do we have some data, or a trailing empty field?
+            if (baos.size() > 0 // we have some data
+                    || push     // we've started a field
+                    || state == EMBEDDEDQUOTE // Just seen ""
+                ) {
                 list.add(baos.toString());
             }
         }
-        return (String[]) list.toArray(new String[]{});
+        return (String[]) list.toArray(new String[list.size()]);
     }
 
     private static boolean isDelimOrEOL(char delim, int ch) {

test/src/org/apache/jmeter/save/TestCSVSaveService.java

@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * 
+ */
+
+package org.apache.jmeter.save;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.StringReader;
+
+import org.apache.jmeter.junit.JMeterTestCase;
+
+public class TestCSVSaveService extends JMeterTestCase {
+
+	public TestCSVSaveService(String name) {
+		super(name);
+	}
+	
+	private void checkSplitString(String input, char delim, String []expected) throws Exception {
+        String out[] = CSVSaveService.csvSplitString(input, delim);	    
+        checkStrings(expected, out);
+	}
+
+    private void checkStrings(String[] expected, String[] out) {
+        assertEquals("Incorrect number of strings returned",expected.length, out.length);
+        for(int i = 0; i < out.length; i++){
+           assertEquals("Incorrect entry returned",expected[i], out[i]);
+        }
+    }
+	
+	// This is what JOrphanUtils.split() does
+	public void testSplitEmpty() throws Exception {
+        checkSplitString("",         ',', new String[]{});    
+	}
+	
+	// These tests should agree with those for JOrphanUtils.split() as far as possible
+	
+	public void testSplitUnquoted() throws Exception {
+        checkSplitString("a",         ',', new String[]{"a"});
+        checkSplitString("a,bc,d,e", ',', new String[]{"a","bc","d","e"});
+        checkSplitString(",bc,d,e",  ',', new String[]{"","bc","d","e"});
+        checkSplitString("a,,d,e",   ',', new String[]{"a","","d","e"});
+        checkSplitString("a,bc, ,e", ',', new String[]{"a","bc"," ","e"});
+        checkSplitString("a,bc,d, ", ',', new String[]{"a","bc","d"," "});
+        checkSplitString("a,bc,d,",  ',', new String[]{"a","bc","d",""});
+        checkSplitString("a,bc,,",   ',', new String[]{"a","bc","",""});
+        checkSplitString("a,,,",     ',', new String[]{"a","","",""});
+        checkSplitString("a,bc,d,\n",',', new String[]{"a","bc","d",""});
+    }
+
+    public void testSplitQuoted() throws Exception {
+        checkSplitString("a,bc,d,e",     ',', new String[]{"a","bc","d","e"});
+        checkSplitString(",bc,d,e",      ',', new String[]{"","bc","d","e"});
+        checkSplitString("\"\",bc,d,e",  ',', new String[]{"","bc","d","e"});
+        checkSplitString("a,,d,e",       ',', new String[]{"a","","d","e"});
+        checkSplitString("a,\"\",d,e",   ',', new String[]{"a","","d","e"});
+        checkSplitString("a,bc, ,e",     ',', new String[]{"a","bc"," ","e"});
+        checkSplitString("a,bc,\" \",e", ',', new String[]{"a","bc"," ","e"});
+        checkSplitString("a,bc,d, ",     ',', new String[]{"a","bc","d"," "});
+        checkSplitString("a,bc,d,\" \"", ',', new String[]{"a","bc","d"," "});
+        checkSplitString("a,bc,d,",      ',', new String[]{"a","bc","d",""});
+        checkSplitString("a,bc,d,\"\"",  ',', new String[]{"a","bc","d",""});
+        checkSplitString("a,bc,d,\"\"\n",',', new String[]{"a","bc","d",""});
+    }
+
+    public void testSplitBadQuote() throws Exception {
+        try {
+            checkSplitString("a\"b",',',null);
+            fail("Should have generated IOException");
+        } catch (IOException e) {
+        }
+    }
+    
+    public void testSplitMultiLine() throws Exception  {
+        String line="a,,\"c\nd\",e\n,,f,g,";
+        String[] out;
+        BufferedReader br = new BufferedReader(new StringReader(line));
+        out = CSVSaveService.csvReadFile(br, ',');
+        checkStrings(new String[]{"a","","c\nd","e"}, out);
+        out = CSVSaveService.csvReadFile(br, ',');
+        checkStrings(new String[]{"","","f","g",""}, out);
+        assertEquals("Expected to be at EOF",-1,br.read());
+        // Empty strings at EOF
+        out = CSVSaveService.csvReadFile(br, ',');
+        checkStrings(new String[]{}, out);
+        out = CSVSaveService.csvReadFile(br, ',');
+        checkStrings(new String[]{}, out);
+    }
+}

xdocs/changes.xml

@@ -185,6 +185,7 @@ These are implemented in the AbstractTestElement class which all elements should
 <li>Fix potential thread safety issue in JMeterThread class</li>
 <li>Mailer Visualiser - fix parsing of multiple e-mail address when using Test button</li>
 <li>Bug 46435 - More verbose error msg for error 501 (Proxy Server)</li>
+<li>Bug 46491 - Incorrect value for the last variable in "CSV Data Set Config" (error in processing quoted strings)</li>
 </ul>
 
 <h3>Improvements</h3>