CSV-114 CSVFormat constructor should reject a header array with duplicate entries

sebbASF · sebbASF · commit 35d101c2adec · 2014-05-07T17:27:31.000Z
git-svn-id: https://svn.apache.org/repos/asf/commons/proper/csv/trunk@1593076 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -40,6 +40,7 @@ The <action> type attribute can be add,update,fix,remove.
   <body>
 
     <release version="1.0" date="TBD" description="First release">
+        <action issue="CSV-114" type="fix" dev="sebb">CSVFormat constructor should reject a header array with duplicate entries</action>
         <action issue="CSV-112" type="fix" dev="britter">HeaderMap is inconsistent when it is parsed from an input with duplicate columns names</action>
         <action issue="CSV-111" type="fix" dev="ggregory">CSVRecord.toMap() fails if row length shorter than header length</action>
         <action issue="CSV-106" type="fix" dev="ggregory">CSVFormat.format allways append null</action>
diff --git a/src/main/java/org/apache/commons/csv/CSVFormat.java b/src/main/java/org/apache/commons/csv/CSVFormat.java
@@ -155,7 +155,7 @@ public final class CSVFormat implements Serializable {
     private final boolean ignoreEmptyLines;
     private final String recordSeparator; // for outputs
     private final String nullString; // the string to be used for null values
-    private final String[] header;
+    private final String[] header; // array of header column names
     private final boolean skipHeaderRecord;
 
     /**
@@ -310,7 +310,17 @@ private CSVFormat(final char delimiter, final Character quoteChar,
         this.ignoreEmptyLines = ignoreEmptyLines;
         this.recordSeparator = recordSeparator;
         this.nullString = nullString;
-        this.header = header == null ? null : header.clone();
+        if (header == null) {
+        	this.header = null;
+        } else {
+        	Set<String> dupCheck = new HashSet<String>();
+        	for(String hdr : header) {
+        		if (!dupCheck.add(hdr)) {
+        			throw new IllegalArgumentException("The header contains a duplicate entry: '" + hdr + "' in " + Arrays.toString(header));
+        		}
+        	}
+            this.header = header.clone();        	
+        }
         this.skipHeaderRecord = skipHeaderRecord;
     }
 
@@ -658,13 +668,6 @@ void validate() throws IllegalStateException {
             throw new IllegalStateException("No quotes mode set but no escape character is set");
         }
 
-        if (header != null) {
-            final Set<String> set = new HashSet<String>(header.length);
-            set.addAll(Arrays.asList(header));
-            if (set.size() != header.length) {
-                throw new IllegalStateException("The header contains duplicate names: " + Arrays.toString(header));
-            }
-        }
     }
 
     /**
diff --git a/src/test/java/org/apache/commons/csv/CSVFormatTest.java b/src/test/java/org/apache/commons/csv/CSVFormatTest.java
@@ -61,7 +61,7 @@ public void testDelimiterSameAsEscapeThrowsException() {
         CSVFormat.DEFAULT.withDelimiter('!').withEscape('!').validate();
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testDuplicateHeaderElements() {
         CSVFormat.DEFAULT.withHeader("A", "A").validate();
     }

Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ public void testDelimiterSameAsEscapeThrowsException() {`
`61`	`61`	`CSVFormat.DEFAULT.withDelimiter('!').withEscape('!').validate();`
`62`	`62`	`}`
`63`	`63`
`64`		`- @Test(expected = IllegalStateException.class)`
	`64`	`+ @Test(expected = IllegalArgumentException.class)`
`65`	`65`	`public void testDuplicateHeaderElements() {`
`66`	`66`	`CSVFormat.DEFAULT.withHeader("A", "A").validate();`
`67`	`67`	`}`