[JENKINS-33600] - User#isIdOrFullnameAllowed() should be tolerant aga…

…inst null parameters (#2406)

* [JENKINS-33600] - User#isIdOrFullnameAllowed() should be tolerant against null parameters

* [JENKINS-33600] - Add the follow-up TODO

core/src/main/java/hudson/model/User.java

@@ -88,6 +88,7 @@
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import org.apache.commons.lang.StringUtils;
 
 /**
  * Represents a user.
@@ -702,10 +703,16 @@ private static File getRootDir() {
      * prevent anyone from logging in as these users. Therefore, we prevent
      * saving a User with one of these ids.
      *
-     * @return true if the username or fullname is valid
+     * @param id ID to be checked
+     * @return {@code true} if the username or fullname is valid.
+     *      For {@code null} or blank IDs returns {@code false}.
      * @since 1.600
      */
-    public static boolean isIdOrFullnameAllowed(String id) {
+    public static boolean isIdOrFullnameAllowed(@CheckForNull String id) {
+        //TODO: StringUtils.isBlank() checks the null falue, but FindBugs is not smart enough. Remove it later
+        if (id == null || StringUtils.isBlank(id)) {
+            return false;
+        }
         for (String invalidId : ILLEGAL_PERSISTED_USERNAMES) {
             if (id.equalsIgnoreCase(invalidId))
                 return false;

core/src/test/java/hudson/model/UserTest.java

@@ -0,0 +1,46 @@
+package hudson.model;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.assertThat;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2016 Oleg Nenashev.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/**
+ * Unit tests for the {@link User} class.
+ * @author Oleg Nenashev
+ */
+public class UserTest {
+
+    @Test
+    @Issue("JENKINS-33600")
+    public void blankIdsOrFullNamesShouldNotBeAllowed() {
+        assertThat("Null user IDs should not be allowed", User.isIdOrFullnameAllowed(null), is(false));
+        assertThat("Empty user IDs should not be allowed", User.isIdOrFullnameAllowed(""), is(false));
+        assertThat("Blank user IDs should not be allowed", User.isIdOrFullnameAllowed("      "), is(false));
+    }
+
+}