From 549e888cd7dddd2aad922cac7952fa49e7c20691 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 11 May 2015 14:20:19 +0200 Subject: [PATCH] CustomBooleanEditor avoids potential NPE in case of allowEmpty=false Issue: SPR-13010 --- .../propertyeditors/CustomBooleanEditor.java | 15 +++---- .../propertyeditors/CustomEditorTests.java | 39 ++++++++++++++----- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/propertyeditors/CustomBooleanEditor.java b/spring-beans/src/main/java/org/springframework/beans/propertyeditors/CustomBooleanEditor.java index 8b1243546b..1bdd2c7630 100644 --- a/spring-beans/src/main/java/org/springframework/beans/propertyeditors/CustomBooleanEditor.java +++ b/spring-beans/src/main/java/org/springframework/beans/propertyeditors/CustomBooleanEditor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -95,6 +95,7 @@ public class CustomBooleanEditor extends PropertyEditorSupport { this.allowEmpty = allowEmpty; } + @Override public void setAsText(String text) throws IllegalArgumentException { String input = (text != null ? text.trim() : null); @@ -102,20 +103,20 @@ public class CustomBooleanEditor extends PropertyEditorSupport { // Treat empty String as null value. setValue(null); } - else if (this.trueString != null && input.equalsIgnoreCase(this.trueString)) { + else if (this.trueString != null && this.trueString.equalsIgnoreCase(input)) { setValue(Boolean.TRUE); } - else if (this.falseString != null && input.equalsIgnoreCase(this.falseString)) { + else if (this.falseString != null && this.falseString.equalsIgnoreCase(input)) { setValue(Boolean.FALSE); } else if (this.trueString == null && - (input.equalsIgnoreCase(VALUE_TRUE) || input.equalsIgnoreCase(VALUE_ON) || - input.equalsIgnoreCase(VALUE_YES) || input.equals(VALUE_1))) { + (VALUE_TRUE.equalsIgnoreCase(input) || VALUE_ON.equalsIgnoreCase(input) || + VALUE_YES.equalsIgnoreCase(input) || VALUE_1.equals(input))) { setValue(Boolean.TRUE); } else if (this.falseString == null && - (input.equalsIgnoreCase(VALUE_FALSE) || input.equalsIgnoreCase(VALUE_OFF) || - input.equalsIgnoreCase(VALUE_NO) || input.equals(VALUE_0))) { + (VALUE_FALSE.equalsIgnoreCase(input) || VALUE_OFF.equalsIgnoreCase(input) || + VALUE_NO.equalsIgnoreCase(input) || VALUE_0.equals(input))) { setValue(Boolean.FALSE); } else { diff --git a/spring-beans/src/test/java/org/springframework/beans/propertyeditors/CustomEditorTests.java b/spring-beans/src/test/java/org/springframework/beans/propertyeditors/CustomEditorTests.java index c8372a29e4..5f233a6613 100644 --- a/spring-beans/src/test/java/org/springframework/beans/propertyeditors/CustomEditorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/propertyeditors/CustomEditorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -59,7 +59,6 @@ import static org.junit.Assert.*; * @author Rob Harrop * @author Arjen Poutsma * @author Chris Beams - * * @since 10.06.2003 */ public class CustomEditorTests { @@ -302,8 +301,8 @@ public class CustomEditorTests { @Test public void testCustomBooleanEditorWithSpecialTrueAndFalseStrings() throws Exception { - final String trueString = "pechorin"; - final String falseString = "nash"; + String trueString = "pechorin"; + String falseString = "nash"; CustomBooleanEditor editor = new CustomBooleanEditor(trueString, falseString, false); @@ -320,6 +319,14 @@ public class CustomEditorTests { editor.setAsText(falseString.toUpperCase()); assertFalse(((Boolean) editor.getValue()).booleanValue()); assertEquals(falseString, editor.getAsText()); + + try { + editor.setAsText(null); + fail("Should have thrown IllegalArgumentException"); + } + catch (IllegalArgumentException ex) { + // expected + } } @Test @@ -423,7 +430,7 @@ public class CustomEditorTests { assertTrue("Correct bigDecimal value", new BigDecimal("4.5").equals(tb.getBigDecimal())); } - @Test(expected=IllegalArgumentException.class) + @Test(expected = IllegalArgumentException.class) public void testCustomNumberEditorCtorWithNullNumberType() throws Exception { new CustomNumberEditor(null, true); } @@ -543,7 +550,7 @@ public class CustomEditorTests { assertNull(cb.getMyCharacter()); } - @Test(expected=IllegalArgumentException.class) + @Test(expected = IllegalArgumentException.class) public void testCharacterEditorSetAsTextWithStringLongerThanOneCharacter() throws Exception { PropertyEditor charEditor = new CharacterEditor(false); charEditor.setAsText("ColdWaterCanyon"); @@ -562,7 +569,7 @@ public class CustomEditorTests { assertEquals(" ", charEditor.getAsText()); } - @Test(expected=IllegalArgumentException.class) + @Test(expected = IllegalArgumentException.class) public void testCharacterEditorSetAsTextWithNullNotAllowingEmptyAsNull() throws Exception { PropertyEditor charEditor = new CharacterEditor(false); charEditor.setAsText(null); @@ -583,7 +590,7 @@ public class CustomEditorTests { assertEquals("", classEditor.getAsText()); } - @Test(expected=IllegalArgumentException.class) + @Test(expected = IllegalArgumentException.class) public void testClassEditorWithNonExistentClass() throws Exception { PropertyEditor classEditor = new ClassEditor(); classEditor.setAsText("hairdresser.on.Fire"); @@ -685,26 +692,40 @@ public class CustomEditorTests { @Test public void testCustomBooleanEditor() { CustomBooleanEditor editor = new CustomBooleanEditor(false); + editor.setAsText("true"); assertEquals(Boolean.TRUE, editor.getValue()); assertEquals("true", editor.getAsText()); + editor.setAsText("false"); assertEquals(Boolean.FALSE, editor.getValue()); assertEquals("false", editor.getAsText()); + editor.setValue(null); assertEquals(null, editor.getValue()); assertEquals("", editor.getAsText()); + + try { + editor.setAsText(null); + fail("Should have thrown IllegalArgumentException"); + } + catch (IllegalArgumentException ex) { + // expected + } } @Test public void testCustomBooleanEditorWithEmptyAsNull() { CustomBooleanEditor editor = new CustomBooleanEditor(true); + editor.setAsText("true"); assertEquals(Boolean.TRUE, editor.getValue()); assertEquals("true", editor.getAsText()); + editor.setAsText("false"); assertEquals(Boolean.FALSE, editor.getValue()); assertEquals("false", editor.getAsText()); + editor.setValue(null); assertEquals(null, editor.getValue()); assertEquals("", editor.getAsText()); @@ -750,7 +771,7 @@ public class CustomEditorTests { } catch (IllegalArgumentException ex) { // expected - assertTrue(ex.getMessage().indexOf("10") != -1); + assertTrue(ex.getMessage().contains("10")); } }