diff --git a/src/etc/testcases/taskdefs/macrodef.xml b/src/etc/testcases/taskdefs/macrodef.xml
index 4669a6c5f..705a65ff9 100644
--- a/src/etc/testcases/taskdefs/macrodef.xml
+++ b/src/etc/testcases/taskdefs/macrodef.xml
@@ -20,6 +20,26 @@
     <my.echo text="Inner Text"/>
   </target>
 
+  <target name="duplicate.attribute">
+    <macrodef name="my.echo">
+      <attribute name="text"/>
+      <attribute name="text"/>
+      <sequential>
+        <echo>${text}</echo>
+      </sequential>
+    </macrodef>
+  </target>
+
+  <target name="duplicate.element">
+    <macrodef name="my.echo">
+      <element name="text"/>
+      <element name="text"/>
+      <sequential>
+        <text/>
+      </sequential>
+    </macrodef>
+  </target>
+
   <target name="uri">
     <macrodef name="echo" uri="abc">
       <attribute name="text"/>
diff --git a/src/main/org/apache/tools/ant/taskdefs/MacroDef.java b/src/main/org/apache/tools/ant/taskdefs/MacroDef.java
index 6741b1468..8aa801938 100644
--- a/src/main/org/apache/tools/ant/taskdefs/MacroDef.java
+++ b/src/main/org/apache/tools/ant/taskdefs/MacroDef.java
@@ -78,7 +78,7 @@ import org.apache.tools.ant.UnknownElement;
 public class MacroDef extends AntlibDefinition  {
     private NestedSequential nestedSequential;
     private String     name;
-    private List       attributes = new ArrayList();
+    private Map        attributes = new HashMap();
     private Map        elements = new HashMap();
 
     /**
@@ -170,7 +170,7 @@ public class MacroDef extends AntlibDefinition  {
     /**
      * @return the nested Attributes
      */
-    public List getAttributes() {
+    public Map getAttributes() {
         return attributes;
     }
 
@@ -221,7 +221,12 @@ public class MacroDef extends AntlibDefinition  {
             throw new BuildException(
                 "the attribute nested element needed a \"name\" attribute");
         }
-        attributes.add(attribute);
+        if (attributes.get(attribute.getName()) != null) {
+            throw new BuildException(
+                "the attribute " + attribute.getName()
+                + " has already been specified");
+        }
+        attributes.put(attribute.getName(), attribute);
     }
 
     /**
@@ -234,6 +239,11 @@ public class MacroDef extends AntlibDefinition  {
             throw new BuildException(
                 "the element nested element needed a \"name\" attribute");
         }
+        if (elements.get(element.getName()) != null) {
+            throw new BuildException(
+                "the element " + element.getName()
+                + " has already been specified");
+        }
         elements.put(element.getName(), element);
     }
 
diff --git a/src/main/org/apache/tools/ant/taskdefs/MacroInstance.java b/src/main/org/apache/tools/ant/taskdefs/MacroInstance.java
index cdab476b0..93e046e1c 100644
--- a/src/main/org/apache/tools/ant/taskdefs/MacroInstance.java
+++ b/src/main/org/apache/tools/ant/taskdefs/MacroInstance.java
@@ -294,9 +294,9 @@ public class MacroInstance extends Task implements DynamicConfigurator {
     public void execute() {
         localProperties = new Hashtable();
         Set copyKeys = new HashSet(map.keySet());
-        for (int i = 0; i < macroDef.getAttributes().size(); ++i) {
-            MacroDef.Attribute attribute =
-                (MacroDef.Attribute) macroDef.getAttributes().get(i);
+        for (Iterator i = macroDef.getAttributes().values().iterator();
+             i.hasNext();) {
+            MacroDef.Attribute attribute = (MacroDef.Attribute) i.next();
             String value = (String) map.get(attribute.getName());
             if (value == null) {
                 value = attribute.getDefault();
diff --git a/src/testcases/org/apache/tools/ant/taskdefs/MacroDefTest.java b/src/testcases/org/apache/tools/ant/taskdefs/MacroDefTest.java
index 24e40d12f..eb57c482e 100644
--- a/src/testcases/org/apache/tools/ant/taskdefs/MacroDefTest.java
+++ b/src/testcases/org/apache/tools/ant/taskdefs/MacroDefTest.java
@@ -78,6 +78,18 @@ public class MacroDefTest extends BuildFileTest {
         expectLog("text", "Inner Text");
     }
 
+    public void testDuplicateAttribute() {
+        expectBuildException(
+            "duplicate.attribute",
+            "the attribute text has already been specified");
+    }
+    
+    public void testDuplicateElement() {
+        expectBuildException(
+            "duplicate.element",
+            "the element text has already been specified");
+    }
+    
     public void testUri() {
         expectLog("uri", "Hello World");
     }