[AA] Bugzilla Bug 60628: Change after code review by @janmaterne
Project: http://git-wip-us.apache.org/repos/asf/ant/repo Commit: http://git-wip-us.apache.org/repos/asf/ant/commit/198d7a2f Tree: http://git-wip-us.apache.org/repos/asf/ant/tree/198d7a2f Diff: http://git-wip-us.apache.org/repos/asf/ant/diff/198d7a2f Branch: refs/heads/master Commit: 198d7a2f449b91d7c559e17e2bc472269a8645ac Parents: 3d21510 Author: Arcadius Ahouansou <arcadius.ahouan...@aexp.com> Authored: Mon Jan 30 22:48:40 2017 +0000 Committer: Arcadius Ahouansou <arcadius.ahouan...@aexp.com> Committed: Mon Jan 30 22:48:40 2017 +0000 ---------------------------------------------------------------------- manual/Tasks/get.html | 11 +++---- src/etc/testcases/taskdefs/get.xml | 8 ++--- src/main/org/apache/tools/ant/taskdefs/Get.java | 32 ++++++-------------- .../org/apache/tools/ant/util/StringUtils.java | 21 +++++++++++++ .../org/apache/tools/ant/taskdefs/GetTest.java | 10 +++--- .../apache/tools/ant/util/StringUtilsTest.java | 27 ++++++++++++++--- 6 files changed, 67 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ant/blob/198d7a2f/manual/Tasks/get.html ---------------------------------------------------------------------- diff --git a/manual/Tasks/get.html b/manual/Tasks/get.html index a6d89e9..e63b59d 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. </tr> <tr> <td valign="top">name</td> - <td valign="top">The name or key of this header.</td> + <td valign="top">The name or key of this header. Cannot be null or empty. Leading and trailing spaces are removed</td> <td align="center" valign="top">Yes</td> </tr> <tr> <td valign="top">value</td> - <td valign="top">The value to assign to the.</td> + <td valign="top">The value to assign to the header. Cannot be null or empty. Leading and trailing spaces are removed</td> <td align="center" valign="top">Yes</td> </tr> </table> @@ -261,10 +261,9 @@ the <a href="input.html">input task</a> to query for a password.</p> <p>With custom HTTP headers</p> <pre> <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> </pre> http://git-wip-us.apache.org/repos/asf/ant/blob/198d7a2f/src/etc/testcases/taskdefs/get.xml ---------------------------------------------------------------------- diff --git a/src/etc/testcases/taskdefs/get.xml b/src/etc/testcases/taskdefs/get.xml index 569d833..188febd 100644 --- a/src/etc/testcases/taskdefs/get.xml +++ b/src/etc/testcases/taskdefs/get.xml @@ -98,21 +98,21 @@ </fail> </target> - <target name="testTwoHeaders"> + <target name="testTwoHeadersAreAddedOK"> <get src="http://www.apache.org/"; dest="get.tmp"> <header name="header1" value="header1Value"/> <header name="header2" value="header2Value"/> </get> </target> - <target name="testEmptyHeaders"> + <target name="testEmptyHeadersAreNeverAdded"> <get src="http://www.apache.org/"; dest="get.tmp"> <header name="" value="headerValue"/> <header name="header2" value=""/> </get> </target> - <target name="testDuplicateHeaderNames"> + <target name="testThatWhenMoreThanOneHeaderHaveSameNameOnlyLastOneIsAdded"> <get src="http://www.apache.org/"; dest="get.tmp"> <header name="header1" value="headerValue1"/> <header name="header1" value="headerValue2"/> @@ -120,7 +120,7 @@ </get> </target> - <target name="testHeaderSpacesTrimmed"> + <target name="testHeaderSpaceTrimmed"> <get src="http://www.apache.org/"; dest="get.tmp"> <header name=" header1 " value=" headerValue1 "/> </get> http://git-wip-us.apache.org/repos/asf/ant/blob/198d7a2f/src/main/org/apache/tools/ant/taskdefs/Get.java ---------------------------------------------------------------------- diff --git a/src/main/org/apache/tools/ant/taskdefs/Get.java b/src/main/org/apache/tools/ant/taskdefs/Get.java index 2200bd8..674a535 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<String, String> 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<String, String> 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); http://git-wip-us.apache.org/repos/asf/ant/blob/198d7a2f/src/main/org/apache/tools/ant/util/StringUtils.java ---------------------------------------------------------------------- diff --git a/src/main/org/apache/tools/ant/util/StringUtils.java b/src/main/org/apache/tools/ant/util/StringUtils.java index 04f1ce8..6ee9c45 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<CharSequence,?,String> 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; + } + } http://git-wip-us.apache.org/repos/asf/ant/blob/198d7a2f/src/tests/junit/org/apache/tools/ant/taskdefs/GetTest.java ---------------------------------------------------------------------- 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 e18fdcd..fb78937 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()); } http://git-wip-us.apache.org/repos/asf/ant/blob/198d7a2f/src/tests/junit/org/apache/tools/ant/util/StringUtilsTest.java ---------------------------------------------------------------------- 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 d2187c4..612c6ec 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 ")); + } }
