S3 APIs: Remove the dependency to com.jamesmurty.utils:java-xmlbuilder (#98)

XMLBuilder is a very nice fluent API for building XML documents.
However, it is only used by a tiny portion of jclouds: the s3 api. The
use of the XMLBuilder class requires a dependency to
com.jamesmurty.utils:java-xmlbuilder jar and a transitive
dependency to the net.iharder:base64 jar (superseded by
java.util.Base64 in java 8). They are 18kb each approximately and they
not OSGi compatible. They are not huge, but they represent more API
surface and more things to change when trying to use jclouds in an OSGi
context (they need to be replaced by OSGi compatible bundles like
org.apache.servicemix.bundles.java-xmlbuilder).

This commit replaces the use of XMLBuilder by a direct use of the
javax.xml and org.w3c.dom APIs.

I hope retesting will be minimal, and most of this code is covered by
unit tests.

apis/s3/pom.xml

@@ -40,10 +40,6 @@
   </properties>
 
   <dependencies>
-    <dependency>
-      <groupId>com.jamesmurty.utils</groupId>
-      <artifactId>java-xmlbuilder</artifactId>
-    </dependency>
     <dependency>
       <groupId>org.apache.jclouds.api</groupId>
       <artifactId>sts</artifactId>

apis/s3/src/main/java/org/jclouds/s3/binders/BindACLToXMLPayload.java

@@ -16,33 +16,34 @@
  */
 package org.jclouds.s3.binders;
 
-import java.util.Properties;
+import static org.jclouds.s3.binders.BindBucketLoggingToXmlPayload.addGrants;
+import static org.jclouds.s3.binders.XMLHelper.asString;
+import static org.jclouds.s3.binders.XMLHelper.createDocument;
+import static org.jclouds.s3.binders.XMLHelper.elem;
+import static org.jclouds.s3.binders.XMLHelper.elemWithText;
 
 import javax.inject.Singleton;
 import javax.ws.rs.core.MediaType;
 import javax.xml.parsers.FactoryConfigurationError;
 import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.TransformerException;
 
 import org.jclouds.http.HttpRequest;
 import org.jclouds.rest.Binder;
 import org.jclouds.s3.domain.AccessControlList;
 import org.jclouds.s3.reference.S3Constants;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
 
 import com.google.common.base.Throwables;
-import com.jamesmurty.utils.XMLBuilder;
-
-import static org.jclouds.s3.binders.BindBucketLoggingToXmlPayload.addGrants;
 
 @Singleton
 public class BindACLToXMLPayload implements Binder {
    @Override
    public <R extends HttpRequest> R bindToRequest(R request, Object payload) {
       AccessControlList from = (AccessControlList) payload;
-      Properties outputProperties = new Properties();
-      outputProperties.put(javax.xml.transform.OutputKeys.OMIT_XML_DECLARATION, "yes");
       try {
-         String stringPayload = generateBuilder(from).asString(outputProperties);
+         request.setPayload(generatePayload(from));
-         request.setPayload(stringPayload);
          request.getPayload().getContentMetadata().setContentType(MediaType.TEXT_XML);
          return request;
       } catch (Exception e) {
@@ -51,18 +52,22 @@ public class BindACLToXMLPayload implements Binder {
       }
    }
 
-   protected XMLBuilder generateBuilder(AccessControlList acl) throws ParserConfigurationException,
+   protected String generatePayload(AccessControlList acl)
-         FactoryConfigurationError {
+         throws ParserConfigurationException, FactoryConfigurationError, TransformerException {
-      XMLBuilder rootBuilder = XMLBuilder.create("AccessControlPolicy").attr("xmlns",
+      Document document = createDocument();
-            S3Constants.S3_REST_API_XML_NAMESPACE);
+      Element rootNode = elem(document, "AccessControlPolicy", document);
+      rootNode.setAttribute("xmlns", S3Constants.S3_REST_API_XML_NAMESPACE);
       if (acl.getOwner() != null) {
-         XMLBuilder ownerBuilder = rootBuilder.elem("Owner");
+         Element ownerNode = elem(rootNode, "Owner", document);
-         ownerBuilder.elem("ID").text(acl.getOwner().getId());
+         elemWithText(ownerNode, "ID", acl.getOwner().getId(), document);
-         if (acl.getOwner().getDisplayName() != null) {
+         String displayName = acl.getOwner().getDisplayName();
-            ownerBuilder.elem("DisplayName").text(acl.getOwner().getDisplayName());
+         if (displayName != null) {
+            elemWithText(ownerNode, "DisplayName", displayName, document);
          }
       }
-      addGrants(rootBuilder.elem("AccessControlList"), acl.getGrants());
+      addGrants(elem(rootNode, "AccessControlList", document),
-      return rootBuilder;
+                acl.getGrants(),
+                document);
+      return asString(document);
    }
 }

apis/s3/src/main/java/org/jclouds/s3/binders/BindBucketLoggingToXmlPayload.java

@@ -16,13 +16,18 @@
  */
 package org.jclouds.s3.binders;
 
+import static org.jclouds.s3.binders.XMLHelper.asString;
+import static org.jclouds.s3.binders.XMLHelper.createDocument;
+import static org.jclouds.s3.binders.XMLHelper.elem;
+import static org.jclouds.s3.binders.XMLHelper.elemWithText;
+
 import java.util.Collection;
-import java.util.Properties;
 
 import javax.inject.Singleton;
 import javax.ws.rs.core.MediaType;
 import javax.xml.parsers.FactoryConfigurationError;
 import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.TransformerException;
 
 import org.jclouds.http.HttpRequest;
 import org.jclouds.rest.Binder;
@@ -32,20 +37,18 @@ import org.jclouds.s3.domain.AccessControlList.Grant;
 import org.jclouds.s3.domain.AccessControlList.GroupGrantee;
 import org.jclouds.s3.domain.BucketLogging;
 import org.jclouds.s3.reference.S3Constants;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
 
 import com.google.common.base.Throwables;
-import com.jamesmurty.utils.XMLBuilder;
 
 @Singleton
 public class BindBucketLoggingToXmlPayload implements Binder {
    @Override
    public <R extends HttpRequest> R bindToRequest(R request, Object payload) {
       BucketLogging from = (BucketLogging) payload;
-      Properties outputProperties = new Properties();
-      outputProperties.put(javax.xml.transform.OutputKeys.OMIT_XML_DECLARATION, "yes");
       try {
-         String stringPayload = generateBuilder(from).asString(outputProperties);
+         request.setPayload(generatePayload(from));
-         request.setPayload(stringPayload);
          request.getPayload().getContentMetadata().setContentType(MediaType.TEXT_XML);
          return request;
       } catch (Exception e) {
@@ -54,36 +57,41 @@ public class BindBucketLoggingToXmlPayload implements Binder {
       }
    }
 
-   protected XMLBuilder generateBuilder(BucketLogging bucketLogging) throws ParserConfigurationException,
+   private String generatePayload(BucketLogging bucketLogging)
-         FactoryConfigurationError {
+         throws ParserConfigurationException, FactoryConfigurationError, TransformerException {
-      XMLBuilder rootBuilder = XMLBuilder.create("BucketLoggingStatus")
+      Document document = createDocument();
-            .attr("xmlns", S3Constants.S3_REST_API_XML_NAMESPACE).elem("LoggingEnabled");
+      Element rootNode = elem(document, "BucketLoggingStatus", document);
-      rootBuilder.elem("TargetBucket").text(bucketLogging.getTargetBucket());
+      rootNode.setAttribute("xmlns", S3Constants.S3_REST_API_XML_NAMESPACE);
-      rootBuilder.elem("TargetPrefix").text(bucketLogging.getTargetPrefix());
+      Element loggingNode = elem(rootNode, "LoggingEnabled", document);
-      addGrants(rootBuilder.elem("TargetGrants"), bucketLogging.getTargetGrants());
+      elemWithText(loggingNode, "TargetBucket", bucketLogging.getTargetBucket(), document);
-      return rootBuilder;
+      elemWithText(loggingNode, "TargetPrefix", bucketLogging.getTargetPrefix(), document);
+      addGrants(elem(loggingNode, "TargetGrants", document),
+                bucketLogging.getTargetGrants(),
+                document);
+      return asString(document);
    }
 
-   static void addGrants(XMLBuilder grantsBuilder, Collection<Grant> grants) {
+   static void addGrants(Element grantsNode, Collection<Grant> grants, Document document) {
       for (Grant grant : grants) {
-         XMLBuilder grantBuilder = grantsBuilder.elem("Grant");
+         Element grantNode = elem(grantsNode, "Grant", document);
-         XMLBuilder granteeBuilder = grantBuilder.elem("Grantee").attr("xmlns:xsi",
+         Element granteeNode = elem(grantNode, "Grantee", document);
-               "http://www.w3.org/2001/XMLSchema-instance");
+         granteeNode.setAttribute("xmlns:xsi", "http://www.w3.org/2001/XMLSchema-instance");
 
          if (grant.getGrantee() instanceof GroupGrantee) {
-            granteeBuilder.attr("xsi:type", "Group").elem("URI").text(grant.getGrantee().getIdentifier());
+            granteeNode.setAttribute("xsi:type", "Group");
+            elemWithText(granteeNode, "URI", grant.getGrantee().getIdentifier(), document);
          } else if (grant.getGrantee() instanceof CanonicalUserGrantee) {
             CanonicalUserGrantee grantee = (CanonicalUserGrantee) grant.getGrantee();
-            granteeBuilder.attr("xsi:type", "CanonicalUser").elem("ID").text(grantee.getIdentifier());
+            granteeNode.setAttribute("xsi:type", "CanonicalUser");
+            elemWithText(granteeNode, "ID", grantee.getIdentifier(), document);
             if (grantee.getDisplayName() != null) {
-               granteeBuilder.elem("DisplayName").text(grantee.getDisplayName());
+               elemWithText(granteeNode, "DisplayName", grantee.getDisplayName(), document);
             }
          } else if (grant.getGrantee() instanceof EmailAddressGrantee) {
-            granteeBuilder.attr("xsi:type", "AmazonCustomerByEmail").elem("EmailAddress")
+            granteeNode.setAttribute("xsi:type", "AmazonCustomerByEmail");
-                  .text(grant.getGrantee().getIdentifier());
+            elemWithText(granteeNode, "EmailAddress", grant.getGrantee().getIdentifier(), document);
          }
-         grantBuilder.elem("Permission").text(grant.getPermission());
+         elemWithText(grantNode, "Permission", grant.getPermission(), document);
       }
    }
-
 }

apis/s3/src/main/java/org/jclouds/s3/binders/BindIterableAsPayloadToDeleteRequest.java

@@ -20,22 +20,24 @@ import static com.google.common.base.Charsets.UTF_8;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.hash.Hashing.md5;
-
+import static org.jclouds.s3.binders.XMLHelper.asString;
-import java.util.Properties;
+import static org.jclouds.s3.binders.XMLHelper.createDocument;
+import static org.jclouds.s3.binders.XMLHelper.elem;
+import static org.jclouds.s3.binders.XMLHelper.elemWithText;
 
 import javax.ws.rs.core.MediaType;
 import javax.xml.parsers.ParserConfigurationException;
-import javax.xml.transform.OutputKeys;
 import javax.xml.transform.TransformerException;
 
 import com.google.common.base.Throwables;
 import com.google.common.collect.Iterables;
-import com.jamesmurty.utils.XMLBuilder;
 
 import org.jclouds.http.HttpRequest;
 import org.jclouds.io.Payload;
 import org.jclouds.io.Payloads;
 import org.jclouds.rest.Binder;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
 
 public class BindIterableAsPayloadToDeleteRequest implements Binder {
 
@@ -51,15 +53,14 @@ public class BindIterableAsPayloadToDeleteRequest implements Binder {
 
       String content;
       try {
-         XMLBuilder rootBuilder = XMLBuilder.create("Delete");
+         Document document = createDocument();
+         Element rootNode = elem(document, "Delete", document);
          for (String key : keys) {
-            rootBuilder.elem("Object").elem("Key").text(key);
+            Element objectNode = elem(rootNode, "Object", document);
+            elemWithText(objectNode, "Key", key, document);
          }
 
-         Properties outputProperties = new Properties();
+         content = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" + asString(document);
-         outputProperties.put(OutputKeys.OMIT_XML_DECLARATION, "yes");
-         content = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
-                rootBuilder.asString(outputProperties);
       } catch (ParserConfigurationException | TransformerException pce) {
          throw Throwables.propagate(pce);
       }

apis/s3/src/main/java/org/jclouds/s3/binders/XMLHelper.java

@@ -0,0 +1,123 @@
+/*
+ * 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.jclouds.s3.binders;
+
+import java.io.StringWriter;
+
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.FactoryConfigurationError;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
+
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+
+final class XMLHelper {
+   static Document createDocument()
+         throws ParserConfigurationException, FactoryConfigurationError {
+      DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+      factory.setNamespaceAware(true);
+      disableExternalEntityParsing(factory);
+      return factory.newDocumentBuilder().newDocument();
+   }
+
+   /**
+    * Explicitly enable or disable the 'external-general-entities' and
+    * 'external-parameter-entities' features of the underlying
+    * DocumentBuilderFactory.
+    *
+    * TODO This is a naive approach that simply tries to apply all known
+    * feature name/URL values in turn until one succeeds, or none do.
+    *
+    * @param factory
+    * factory which will have external general and parameter entities enabled
+    * or disabled.
+    */
+   private static void disableExternalEntityParsing(DocumentBuilderFactory factory) {
+      // Feature list drawn from:
+      // https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing
+
+      /* Enable or disable external general entities */
+      String[] externalGeneralEntitiesFeatures = {
+            // General
+            "http://xml.org/sax/features/external-general-entities",
+            // Xerces 1
+            "http://xerces.apache.org/xerces-j/features.html#external-general-entities",
+            // Xerces 2
+            "http://xerces.apache.org/xerces2-j/features.html#external-general-entities",
+      };
+      disableFeatures(factory, externalGeneralEntitiesFeatures);
+
+      /* Enable or disable external parameter entities */
+      String[] externalParameterEntitiesFeatures = {
+            // General
+            "http://xml.org/sax/features/external-parameter-entities",
+            // Xerces 1
+            "http://xerces.apache.org/xerces-j/features.html#external-parameter-entities",
+            // Xerces 2
+            "http://xerces.apache.org/xerces2-j/features.html#external-parameter-entities",
+      };
+      disableFeatures(factory, externalParameterEntitiesFeatures);
+   }
+
+   private static void disableFeatures(DocumentBuilderFactory factory, String[] features) {
+      for (String feature : features) {
+         try {
+            factory.setFeature(feature, false);
+            break;
+         } catch (ParserConfigurationException e) {
+         }
+      }
+   }
+
+   static void elemWithText(Element node, String name, String text, Document document) {
+      text(elem(node, name, document),
+           text,
+           document);
+   }
+
+   static Element elem(Node node, String name, Document document) {
+      Element newNode = document.createElement(name);
+      node.appendChild(newNode);
+      return newNode;
+   }
+
+   private static void text(Element node, String value, Document document) {
+      if (value == null) {
+          // null text values cause exceptions on subsequent call to
+          // Transformer to render document, so fail-fast here on bad data.
+          throw new IllegalArgumentException("Illegal null text value");
+      }
+      node.appendChild(document.createTextNode(value));
+   }
+
+   /** Serializes the XML document into a string. */
+   static String asString(Document document) throws TransformerException {
+      StringWriter writer = new StringWriter();
+      Transformer serializer = TransformerFactory.newInstance().newTransformer();
+      serializer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes");
+      serializer.transform(new DOMSource(document), new StreamResult(writer));
+      return writer.toString();
+   }
+}
+

project/pom.xml

@@ -229,7 +229,6 @@
     <auto-factory.version>0.1-beta1</auto-factory.version>
     <auto-service.version>1.0-rc3</auto-service.version>
     <auto-value.version>1.4.1</auto-value.version>
-    <java-xmlbuilder.version>1.2</java-xmlbuilder.version>
 
     <jetty.version>8.1.8.v20121106</jetty.version>
     <javax.ws.rs-api.version>2.0.1</javax.ws.rs-api.version>
@@ -306,11 +305,6 @@
         <artifactId>auto-value</artifactId>
         <version>${auto-value.version}</version>
       </dependency>
-      <dependency>
-        <groupId>com.jamesmurty.utils</groupId>
-        <artifactId>java-xmlbuilder</artifactId>
-        <version>${java-xmlbuilder.version}</version>
-      </dependency>
 
       <dependency>
         <groupId>org.eclipse.jetty</groupId>