From b83bdcebf0395250b3ae5e7e4af94ff1400493f0 Mon Sep 17 00:00:00 2001 From: Arcadius Ahouansou Date: Mon, 23 Jan 2017 22:46:06 +0000 Subject: [PATCH 1/3] [AA] Bugzilla Bug 60628 - Ant Get Task To Accept Arbitrary Header --- manual/Tasks/get.html | 34 +++++++++++++++ src/etc/testcases/taskdefs/get.xml | 28 ++++++++++++ .../org/apache/tools/ant/taskdefs/Get.java | 43 +++++++++++++++++++ .../tools/ant/taskdefs/email/Header.java | 4 +- .../apache/tools/ant/taskdefs/GetTest.java | 36 ++++++++++++++++ 5 files changed, 144 insertions(+), 1 deletion(-) diff --git a/manual/Tasks/get.html b/manual/Tasks/get.html index a2c9b7e59..a6d89e9dd 100644 --- a/manual/Tasks/get.html +++ b/manual/Tasks/get.html @@ -170,6 +170,29 @@ plain text' authentication is used. This is only secure over an HTTPS link. name will be skipped. If the returned name is a relative path, it will be considered relative to the dest attribute.

+

header

+

Any arbitrary number of HTTP headers can be added to a request.
+ The attributes of a nested 

<header/>
node are as follow: +

+ + + + + + + + + + + + + + + + + +
AttributeDescriptionRequired
nameThe name or key of this header.Yes
valueThe value to assign to the.Yes
+

Examples

  <get src="http://ant.apache.org/" dest="help/index.html"/>

Gets the index page of http://ant.apache.org/, and stores it in the file help/index.html.

@@ -234,6 +257,17 @@ the input task to query for a password.

<url url="http://ant.apache.org/faq.html"/> </get> + +

With custom HTTP headers

+
+<get src="http://ant.apache.org/index.html" dest="downloads">
+  <header name="header1" value=="headerValue1" />
+  <header name="header2" value=="headerValue2" />
+  <header name="header3" value=="headerValue3" />
+
+</get>
+
+

Gets the index and FAQ pages of http://ant.apache.org/, and stores them in the directory downloads which will be created if necessary.

diff --git a/src/etc/testcases/taskdefs/get.xml b/src/etc/testcases/taskdefs/get.xml index b74e92ab2..569d8333e 100644 --- a/src/etc/testcases/taskdefs/get.xml +++ b/src/etc/testcases/taskdefs/get.xml @@ -98,6 +98,34 @@ + + +
+
+ + + + + +
+
+ + + + + +
+
+
+ + + + + +
+ + + diff --git a/src/main/org/apache/tools/ant/taskdefs/Get.java b/src/main/org/apache/tools/ant/taskdefs/Get.java index 83f3b6bf0..2200bd89f 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Get.java +++ b/src/main/org/apache/tools/ant/taskdefs/Get.java @@ -36,6 +36,7 @@ import org.apache.tools.ant.MagicNames; import org.apache.tools.ant.Main; import org.apache.tools.ant.Project; import org.apache.tools.ant.Task; +import org.apache.tools.ant.taskdefs.email.Header; import org.apache.tools.ant.types.Mapper; import org.apache.tools.ant.types.Resource; import org.apache.tools.ant.types.ResourceCollection; @@ -44,6 +45,8 @@ import org.apache.tools.ant.types.resources.URLProvider; import org.apache.tools.ant.types.resources.URLResource; import org.apache.tools.ant.util.FileNameMapper; import org.apache.tools.ant.util.FileUtils; +import java.util.LinkedHashMap; +import java.util.Map; /** * Gets a particular file from a URL source. @@ -90,6 +93,9 @@ public class Get extends Task { DEFAULT_AGENT_PREFIX + "/" + Main.getShortAntVersion()); + // Store headers as key/value pair without duplicate in keyz + private Map headers = new LinkedHashMap(); + /** * Does the work. * @@ -482,6 +488,35 @@ public class Get extends Task { tryGzipEncoding = b; } + /** + * Add a nested header + * @param header to be added + * + */ + public void addConfiguredHeader(Header header) { + if (header != null) { + String key = trimToNull(header.getName()); + String value = trimToNull(header.getValue()); + if (key != null && value != null) { + this.headers.put(key, value); + } + } + } + + private String trimToNull(String inputString) { + + if (inputString == null) { + return null; + } + + inputString = inputString.trim(); + if ("".equals(inputString)) { + return null; + } + return inputString; + } + + /** * Define the mapper to map source to destination files. * @return a mapper to be configured. @@ -726,6 +761,14 @@ public class Get extends Task { connection.setRequestProperty("Accept-Encoding", GZIP_CONTENT_ENCODING); } + if (!headers.isEmpty()) { + for (final Map.Entry header : headers.entrySet()) { + //we do not log the header value as it may contain sensitive data like passwords + log(String.format("Adding header '%s' ", header.getKey())); + connection.setRequestProperty(header.getKey(), header.getValue()); + } + } + if (connection instanceof HttpURLConnection) { ((HttpURLConnection) connection) .setInstanceFollowRedirects(false); diff --git a/src/main/org/apache/tools/ant/taskdefs/email/Header.java b/src/main/org/apache/tools/ant/taskdefs/email/Header.java index 6bcfb66ff..aaee69352 100644 --- a/src/main/org/apache/tools/ant/taskdefs/email/Header.java +++ b/src/main/org/apache/tools/ant/taskdefs/email/Header.java @@ -19,7 +19,9 @@ package org.apache.tools.ant.taskdefs.email; /** - * Class representing a generic e-mail header. + * Class representing a generic key-value header. + * TODO: This should be moved out of the email package + * * @since Ant 1.7 */ public class Header { diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java index 3e1157d8b..52950c911 100644 --- a/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java @@ -18,6 +18,7 @@ package org.apache.tools.ant.taskdefs; +import org.apache.tools.ant.AntAssert; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.BuildFileRule; import org.junit.After; @@ -25,6 +26,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; /** @@ -103,6 +105,8 @@ public class GetTest { public void test7() { try { buildRule.executeTarget("test7"); + AntAssert.assertNotContains("Adding header", buildRule.getLog()); + fail("userAgent may not be null or empty"); } catch (BuildException ex) { //TODO assert value @@ -119,4 +123,36 @@ public class GetTest { buildRule.executeTarget("testUseTomorrow"); } + @Test + public void testTwoHeaders() { + buildRule.executeTarget("testTwoHeaders"); + String log = buildRule.getLog(); + AntAssert.assertContains("Adding header 'header1'", log); + AntAssert.assertContains("Adding header 'header2'", log); + } + + @Test + public void testEmptyHeadersAreNeverAdded() { + buildRule.executeTarget("testEmptyHeaders"); + AntAssert.assertNotContains("Adding header", buildRule.getLog()); + } + + @Test + public void testThatWhenMoreThanOneHeaderHaveSameNameOnlyLastOneIsAdded() { + buildRule.executeTarget("testDuplicateHeaderNames"); + String log = buildRule.getLog(); + AntAssert.assertContains("Adding header 'header1'", log); + + String firstHeaderLogTrimmed = log.replaceFirst("Adding header ", ""); + String allHeaderLogsTrimmed = log.replaceAll("Adding header ", ""); + + assertEquals("Only one header has been added", firstHeaderLogTrimmed, allHeaderLogsTrimmed); + } + + @Test + public void testHeaderSpaceTrimmed() { + buildRule.executeTarget("testHeaderSpacesTrimmed"); + AntAssert.assertContains("Adding header 'header1'", buildRule.getLog()); + } + } From 3d215101449f63be2c62baf49425a929d758168f Mon Sep 17 00:00:00 2001 From: Arcadius Date: Mon, 23 Jan 2017 23:50:55 +0000 Subject: [PATCH 2/3] [AA] bugzilla-Bug-60628-Ant-Get-Task-To-Accept-Arbitrary-Header : simplifying test --- src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java index 52950c911..e18fdcd14 100644 --- a/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java @@ -143,10 +143,9 @@ public class GetTest { String log = buildRule.getLog(); AntAssert.assertContains("Adding header 'header1'", log); - String firstHeaderLogTrimmed = log.replaceFirst("Adding header ", ""); - String allHeaderLogsTrimmed = log.replaceAll("Adding header ", ""); + int actualHeaderCount = log.split("Adding header ").length - 1; - assertEquals("Only one header has been added", firstHeaderLogTrimmed, allHeaderLogsTrimmed); + assertEquals("Only one header has been added", 1, actualHeaderCount); } @Test From 198d7a2f449b91d7c559e17e2bc472269a8645ac Mon Sep 17 00:00:00 2001 From: Arcadius Ahouansou Date: Mon, 30 Jan 2017 22:48:40 +0000 Subject: [PATCH 3/3] [AA] Bugzilla Bug 60628: Change after code review by @janmaterne --- manual/Tasks/get.html | 11 +++---- src/etc/testcases/taskdefs/get.xml | 8 ++--- .../org/apache/tools/ant/taskdefs/Get.java | 32 ++++++------------- .../apache/tools/ant/util/StringUtils.java | 21 ++++++++++++ .../apache/tools/ant/taskdefs/GetTest.java | 10 +++--- .../tools/ant/util/StringUtilsTest.java | 27 +++++++++++++--- 6 files changed, 67 insertions(+), 42 deletions(-) diff --git a/manual/Tasks/get.html b/manual/Tasks/get.html index a6d89e9dd..e63b59df8 100644 --- a/manual/Tasks/get.html +++ b/manual/Tasks/get.html @@ -183,12 +183,12 @@ plain text' authentication is used. This is only secure over an HTTPS link. name - The name or key of this header. + The name or key of this header. Cannot be null or empty. Leading and trailing spaces are removed Yes value - The value to assign to the. + The value to assign to the header. Cannot be null or empty. Leading and trailing spaces are removed Yes @@ -261,10 +261,9 @@ the input task to query for a password.

With custom HTTP headers

 <get src="http://ant.apache.org/index.html" dest="downloads">
-  <header name="header1" value=="headerValue1" />
-  <header name="header2" value=="headerValue2" />
-  <header name="header3" value=="headerValue3" />
-
+  <header name="header1" value="headerValue1" />
+  <header name="header2" value="headerValue2" />
+  <header name="header3" value="headerValue3" />
 </get>
diff --git a/src/etc/testcases/taskdefs/get.xml b/src/etc/testcases/taskdefs/get.xml index 569d8333e..188febddc 100644 --- a/src/etc/testcases/taskdefs/get.xml +++ b/src/etc/testcases/taskdefs/get.xml @@ -98,21 +98,21 @@ - +
- +
- +
@@ -120,7 +120,7 @@ - +
diff --git a/src/main/org/apache/tools/ant/taskdefs/Get.java b/src/main/org/apache/tools/ant/taskdefs/Get.java index 2200bd89f..674a535e1 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Get.java +++ b/src/main/org/apache/tools/ant/taskdefs/Get.java @@ -45,6 +45,8 @@ import org.apache.tools.ant.types.resources.URLProvider; import org.apache.tools.ant.types.resources.URLResource; import org.apache.tools.ant.util.FileNameMapper; import org.apache.tools.ant.util.FileUtils; +import org.apache.tools.ant.util.StringUtils; + import java.util.LinkedHashMap; import java.util.Map; @@ -495,28 +497,14 @@ public class Get extends Task { */ public void addConfiguredHeader(Header header) { if (header != null) { - String key = trimToNull(header.getName()); - String value = trimToNull(header.getValue()); + String key = StringUtils.trimToNull(header.getName()); + String value = StringUtils.trimToNull(header.getValue()); if (key != null && value != null) { this.headers.put(key, value); } } } - private String trimToNull(String inputString) { - - if (inputString == null) { - return null; - } - - inputString = inputString.trim(); - if ("".equals(inputString)) { - return null; - } - return inputString; - } - - /** * Define the mapper to map source to destination files. * @return a mapper to be configured. @@ -761,14 +749,14 @@ public class Get extends Task { connection.setRequestProperty("Accept-Encoding", GZIP_CONTENT_ENCODING); } - if (!headers.isEmpty()) { - for (final Map.Entry header : headers.entrySet()) { - //we do not log the header value as it may contain sensitive data like passwords - log(String.format("Adding header '%s' ", header.getKey())); - connection.setRequestProperty(header.getKey(), header.getValue()); - } + + for (final Map.Entry header : headers.entrySet()) { + //we do not log the header value as it may contain sensitive data like passwords + log(String.format("Adding header '%s' ", header.getKey())); + connection.setRequestProperty(header.getKey(), header.getValue()); } + if (connection instanceof HttpURLConnection) { ((HttpURLConnection) connection) .setInstanceFollowRedirects(false); diff --git a/src/main/org/apache/tools/ant/util/StringUtils.java b/src/main/org/apache/tools/ant/util/StringUtils.java index 04f1ce8c1..6ee9c450d 100644 --- a/src/main/org/apache/tools/ant/util/StringUtils.java +++ b/src/main/org/apache/tools/ant/util/StringUtils.java @@ -306,4 +306,25 @@ public final class StringUtils { private static Collector joining(CharSequence separator) { return separator == null ? Collectors.joining() : Collectors.joining(separator); } + + + /** + * @param inputString String to trim + * @return null if the input string is null or empty or contain only empty spaces. + * It returns the input string without leading and trailing spaces otherwise. + * + */ + public static String trimToNull(String inputString) { + + if (inputString == null) { + return null; + } + + String tmpString = inputString.trim(); + if ("".equals(tmpString)) { + return null; + } + return tmpString; + } + } diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java index e18fdcd14..fb7893707 100644 --- a/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java @@ -124,8 +124,8 @@ public class GetTest { } @Test - public void testTwoHeaders() { - buildRule.executeTarget("testTwoHeaders"); + public void testTwoHeadersAreAddedOK() { + buildRule.executeTarget("testTwoHeadersAreAddedOK"); String log = buildRule.getLog(); AntAssert.assertContains("Adding header 'header1'", log); AntAssert.assertContains("Adding header 'header2'", log); @@ -133,13 +133,13 @@ public class GetTest { @Test public void testEmptyHeadersAreNeverAdded() { - buildRule.executeTarget("testEmptyHeaders"); + buildRule.executeTarget("testEmptyHeadersAreNeverAdded"); AntAssert.assertNotContains("Adding header", buildRule.getLog()); } @Test public void testThatWhenMoreThanOneHeaderHaveSameNameOnlyLastOneIsAdded() { - buildRule.executeTarget("testDuplicateHeaderNames"); + buildRule.executeTarget("testThatWhenMoreThanOneHeaderHaveSameNameOnlyLastOneIsAdded"); String log = buildRule.getLog(); AntAssert.assertContains("Adding header 'header1'", log); @@ -150,7 +150,7 @@ public class GetTest { @Test public void testHeaderSpaceTrimmed() { - buildRule.executeTarget("testHeaderSpacesTrimmed"); + buildRule.executeTarget("testHeaderSpaceTrimmed"); AntAssert.assertContains("Adding header 'header1'", buildRule.getLog()); } diff --git a/src/tests/junit/org/apache/tools/ant/util/StringUtilsTest.java b/src/tests/junit/org/apache/tools/ant/util/StringUtilsTest.java index d2187c4e0..612c6ec3a 100644 --- a/src/tests/junit/org/apache/tools/ant/util/StringUtilsTest.java +++ b/src/tests/junit/org/apache/tools/ant/util/StringUtilsTest.java @@ -17,16 +17,14 @@ */ package org.apache.tools.ant.util; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - import java.util.Arrays; import java.util.Collection; import java.util.Vector; import org.junit.Test; +import static org.junit.Assert.*; + /** * Test for StringUtils */ @@ -195,5 +193,24 @@ public class StringUtilsTest { public void testJoinNullSeparator() { assertEquals("abc", StringUtils.join(Arrays.asList("a", "b", "c"), null)); } - + + @Test + public void testTrimToNullWithNullInput(){ + assertNull(StringUtils.trimToNull(null)); + } + + @Test + public void testTrimToNullWithEmptyInput(){ + assertNull(StringUtils.trimToNull("")); + } + + @Test + public void testTrimToNullWithBlankSpaceInput(){ + assertNull(StringUtils.trimToNull(" ")); + } + + @Test + public void testTrimToNullWithInputPaddedWithSpace(){ + assertEquals("aaBcDeF",StringUtils.trimToNull(" aaBcDeF ")); + } }