[jira] Created: (CXF-2520) wrong handling in setNamespaceMap for AbstractDataBinding

wrong handling in setNamespaceMap for AbstractDataBinding
---------------------------------------------------------

                 Key: CXF-2520
                 URL: https://issues.apache.org/jira/browse/CXF-2520
             Project: CXF
          Issue Type: Bug
          Components: JAXB Databinding
    Affects Versions: 2.2.4
            Reporter: Hasan Hosgel


The Class org.apache.cxf.databinding.AbstractDataBinding has a Bug in the checking of duplicate NamespacePrefixes during the setting of the namespaceMap.

Extraction of class:
    public void setNamespaceMap(Map<String, String> namespaceMap) {
        // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
        if (namespaceMap != null) {
            Set<String> prefixesSoFar = new HashSet<String>();
            for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
                if (prefixesSoFar.contains(mapping.getValue())) {
                    throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
                }
            }
        }
        this.namespaceMap = namespaceMap;
    }

There is no adding method for prefixesSoFar-Set.

My solving idea is:
  /**
   * @param namespaceMap The namespaceMap to set.
   */
  public void setNamespaceMap(Map<String, String> namespaceMap) {
    // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
    if ((namespaceMap == null) || namespaceMap.isEmpty()) {
      throw new IllegalArgumentException("adding a null or empty namespaceMap is not allowed");
    } else {
      Set<String> prefixesSoFar = new HashSet<String>();
      for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
        if (prefixesSoFar.contains(mapping.getValue())) {
          throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
        }
        prefixesSoFar.add(mapping.getValue());
      }
      mapper = new NamespaceMapper(namespaceMap);
    }
  }

This solution is more strict, because it does not allow to add a null or an empty Map.

The same Bug goes for checkNameSpaceMap.
The both method are smelling, because it is copy paste. It can be solved in a smater way :).



--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/CXF-2520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773469#action_12773469 ]

Benson Margulies commented on CXF-2520:
---------------------------------------

Would you be willing to post a formal patch with a cleanup?

> wrong handling in setNamespaceMap for AbstractDataBinding
> ---------------------------------------------------------
>
>                 Key: CXF-2520
>                 URL: https://issues.apache.org/jira/browse/CXF-2520
>             Project: CXF
>          Issue Type: Bug
>          Components: JAXB Databinding
>    Affects Versions: 2.2.4
>            Reporter: Hasan Hosgel
>
> The Class org.apache.cxf.databinding.AbstractDataBinding has a Bug in the checking of duplicate NamespacePrefixes during the setting of the namespaceMap.
> Extraction of class:
>     public void setNamespaceMap(Map<String, String> namespaceMap) {
>         // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
>         if (namespaceMap != null) {
>             Set<String> prefixesSoFar = new HashSet<String>();
>             for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
>                 if (prefixesSoFar.contains(mapping.getValue())) {
>                     throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
>                 }
>             }
>         }
>         this.namespaceMap = namespaceMap;
>     }
> There is no adding method for prefixesSoFar-Set.
> My solving idea is:
>   /**
>    * @param namespaceMap The namespaceMap to set.
>    */
>   public void setNamespaceMap(Map<String, String> namespaceMap) {
>     // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
>     if ((namespaceMap == null) || namespaceMap.isEmpty()) {
>       throw new IllegalArgumentException("adding a null or empty namespaceMap is not allowed");
>     } else {
>       Set<String> prefixesSoFar = new HashSet<String>();
>       for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
>         if (prefixesSoFar.contains(mapping.getValue())) {
>           throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
>         }
>         prefixesSoFar.add(mapping.getValue());
>       }
>       mapper = new NamespaceMapper(namespaceMap);
>     }
>   }
> This solution is more strict, because it does not allow to add a null or an empty Map.
> The same Bug goes for checkNameSpaceMap.
> The both method are smelling, because it is copy paste. It can be solved in a smater way :).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/CXF-2520?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Benson Margulies resolved CXF-2520.
-----------------------------------

       Resolution: Fixed
    Fix Version/s: 2.3

I did this in r833963. I did not reject null, rejecting null was never intended.

> wrong handling in setNamespaceMap for AbstractDataBinding
> ---------------------------------------------------------
>
>                 Key: CXF-2520
>                 URL: https://issues.apache.org/jira/browse/CXF-2520
>             Project: CXF
>          Issue Type: Bug
>          Components: JAXB Databinding
>    Affects Versions: 2.2.4
>            Reporter: Hasan Hosgel
>             Fix For: 2.3
>
>
> The Class org.apache.cxf.databinding.AbstractDataBinding has a Bug in the checking of duplicate NamespacePrefixes during the setting of the namespaceMap.
> Extraction of class:
>     public void setNamespaceMap(Map<String, String> namespaceMap) {
>         // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
>         if (namespaceMap != null) {
>             Set<String> prefixesSoFar = new HashSet<String>();
>             for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
>                 if (prefixesSoFar.contains(mapping.getValue())) {
>                     throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
>                 }
>             }
>         }
>         this.namespaceMap = namespaceMap;
>     }
> There is no adding method for prefixesSoFar-Set.
> My solving idea is:
>   /**
>    * @param namespaceMap The namespaceMap to set.
>    */
>   public void setNamespaceMap(Map<String, String> namespaceMap) {
>     // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
>     if ((namespaceMap == null) || namespaceMap.isEmpty()) {
>       throw new IllegalArgumentException("adding a null or empty namespaceMap is not allowed");
>     } else {
>       Set<String> prefixesSoFar = new HashSet<String>();
>       for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
>         if (prefixesSoFar.contains(mapping.getValue())) {
>           throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
>         }
>         prefixesSoFar.add(mapping.getValue());
>       }
>       mapper = new NamespaceMapper(namespaceMap);
>     }
>   }
> This solution is more strict, because it does not allow to add a null or an empty Map.
> The same Bug goes for checkNameSpaceMap.
> The both method are smelling, because it is copy paste. It can be solved in a smater way :).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/CXF-2520?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Benson Margulies updated CXF-2520:
----------------------------------

    Component/s:     (was: JAXB Databinding)
                 Core

> wrong handling in setNamespaceMap for AbstractDataBinding
> ---------------------------------------------------------
>
>                 Key: CXF-2520
>                 URL: https://issues.apache.org/jira/browse/CXF-2520
>             Project: CXF
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 2.2.4
>            Reporter: Hasan Hosgel
>             Fix For: 2.3
>
>
> The Class org.apache.cxf.databinding.AbstractDataBinding has a Bug in the checking of duplicate NamespacePrefixes during the setting of the namespaceMap.
> Extraction of class:
>     public void setNamespaceMap(Map<String, String> namespaceMap) {
>         // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
>         if (namespaceMap != null) {
>             Set<String> prefixesSoFar = new HashSet<String>();
>             for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
>                 if (prefixesSoFar.contains(mapping.getValue())) {
>                     throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
>                 }
>             }
>         }
>         this.namespaceMap = namespaceMap;
>     }
> There is no adding method for prefixesSoFar-Set.
> My solving idea is:
>   /**
>    * @param namespaceMap The namespaceMap to set.
>    */
>   public void setNamespaceMap(Map<String, String> namespaceMap) {
>     // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
>     if ((namespaceMap == null) || namespaceMap.isEmpty()) {
>       throw new IllegalArgumentException("adding a null or empty namespaceMap is not allowed");
>     } else {
>       Set<String> prefixesSoFar = new HashSet<String>();
>       for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
>         if (prefixesSoFar.contains(mapping.getValue())) {
>           throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
>         }
>         prefixesSoFar.add(mapping.getValue());
>       }
>       mapper = new NamespaceMapper(namespaceMap);
>     }
>   }
> This solution is more strict, because it does not allow to add a null or an empty Map.
> The same Bug goes for checkNameSpaceMap.
> The both method are smelling, because it is copy paste. It can be solved in a smater way :).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/CXF-2520?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Daniel Kulp updated CXF-2520:
-----------------------------

    Fix Version/s:     (was: 2.3)
                   2.2.5

> wrong handling in setNamespaceMap for AbstractDataBinding
> ---------------------------------------------------------
>
>                 Key: CXF-2520
>                 URL: https://issues.apache.org/jira/browse/CXF-2520
>             Project: CXF
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 2.2.4
>            Reporter: Hasan Hosgel
>             Fix For: 2.2.5
>
>
> The Class org.apache.cxf.databinding.AbstractDataBinding has a Bug in the checking of duplicate NamespacePrefixes during the setting of the namespaceMap.
> Extraction of class:
>     public void setNamespaceMap(Map<String, String> namespaceMap) {
>         // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
>         if (namespaceMap != null) {
>             Set<String> prefixesSoFar = new HashSet<String>();
>             for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
>                 if (prefixesSoFar.contains(mapping.getValue())) {
>                     throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
>                 }
>             }
>         }
>         this.namespaceMap = namespaceMap;
>     }
> There is no adding method for prefixesSoFar-Set.
> My solving idea is:
>   /**
>    * @param namespaceMap The namespaceMap to set.
>    */
>   public void setNamespaceMap(Map<String, String> namespaceMap) {
>     // make some checks. This is a map from namespace to prefix, but we want unique prefixes.
>     if ((namespaceMap == null) || namespaceMap.isEmpty()) {
>       throw new IllegalArgumentException("adding a null or empty namespaceMap is not allowed");
>     } else {
>       Set<String> prefixesSoFar = new HashSet<String>();
>       for (Map.Entry<String, String> mapping : namespaceMap.entrySet()) {
>         if (prefixesSoFar.contains(mapping.getValue())) {
>           throw new IllegalArgumentException("Duplicate prefix " + mapping.getValue());
>         }
>         prefixesSoFar.add(mapping.getValue());
>       }
>       mapper = new NamespaceMapper(namespaceMap);
>     }
>   }
> This solution is more strict, because it does not allow to add a null or an empty Map.
> The same Bug goes for checkNameSpaceMap.
> The both method are smelling, because it is copy paste. It can be solved in a smater way :).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.