This is an automated email from the ASF dual-hosted git repository. juanpablo pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/jspwiki.git
commit 00ffc39ea77585cf3f3ac3f62743a29429335efb Author: Juan Pablo Santos RodrÃguez <[email protected]> AuthorDate: Thu Jan 13 11:55:44 2022 +0100 XMLUserDatabase does not allow empty wiki names jdom returns an empty string (that is, not null) for missing attributes, so checking for null at getWikiNames results in unreachable code. Not sure if it's intended or a bug, but given that the UI on the registration form mandates a not empty value for the wiki name, let's follow that when returning wiki names. Also, applied code formatting --- .../org/apache/wiki/auth/user/XMLUserDatabase.java | 44 +++++++++++----------- .../apache/wiki/auth/user/XMLUserDatabaseTest.java | 15 +++----- 2 files changed, 28 insertions(+), 31 deletions(-) diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java index a4d3b87..2411b8a 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java @@ -91,7 +91,7 @@ public class XMLUserDatabase extends AbstractUserDatabase { /** {@inheritDoc} */ @Override - public synchronized void deleteByLoginName( final String loginName ) throws NoSuchPrincipalException, WikiSecurityException { + public synchronized void deleteByLoginName( final String loginName ) throws WikiSecurityException { if( c_dom == null ) { throw new WikiSecurityException( "FATAL: database does not exist" ); } @@ -154,15 +154,15 @@ public class XMLUserDatabase extends AbstractUserDatabase { if ( c_dom == null ) { throw new IllegalStateException( "FATAL: database does not exist" ); } - final SortedSet< Principal > principals = new TreeSet<>(); + final SortedSet< WikiPrincipal > principals = new TreeSet<>(); final NodeList users = c_dom.getElementsByTagName( USER_TAG ); for( int i = 0; i < users.getLength(); i++ ) { - final Element user = (Element) users.item( i ); + final Element user = ( Element )users.item( i ); final String wikiName = user.getAttribute( WIKI_NAME ); - if( wikiName == null ) { - log.warn( "Detected null wiki name in XMLUserDataBase. Check your user database." ); + if( StringUtils.isEmpty( wikiName ) ) { + log.warn( "Detected null or empty wiki name for {} in XMLUserDataBase. Check your user database.", user.getAttribute( LOGIN_NAME ) ); } else { - final Principal principal = new WikiPrincipal( wikiName, WikiPrincipal.WIKI_NAME ); + final WikiPrincipal principal = new WikiPrincipal( wikiName, WikiPrincipal.WIKI_NAME ); principals.add( principal ); } } @@ -202,19 +202,21 @@ public class XMLUserDatabase extends AbstractUserDatabase { factory.setExpandEntityReferences( false ); factory.setIgnoringComments( true ); factory.setNamespaceAware( false ); + //factory.setAttribute( XMLConstants.ACCESS_EXTERNAL_DTD, "" ); + //factory.setAttribute( XMLConstants.ACCESS_EXTERNAL_SCHEMA, "" ); try { c_dom = factory.newDocumentBuilder().parse( c_file ); log.debug( "Database successfully initialized" ); c_lastModified = c_file.lastModified(); c_lastCheck = System.currentTimeMillis(); } catch( final ParserConfigurationException e ) { - log.error( "Configuration error: " + e.getMessage() ); + log.error( "Configuration error: {}", e.getMessage() ); } catch( final SAXException e ) { - log.error( "SAX error: " + e.getMessage() ); + log.error( "SAX error: {}", e.getMessage() ); } catch( final FileNotFoundException e ) { log.info( "User database not found; creating from scratch..." ); } catch( final IOException e ) { - log.error( "IO error: " + e.getMessage() ); + log.error( "IO error: {}", e.getMessage() ); } if( c_dom == null ) { try { @@ -318,7 +320,7 @@ public class XMLUserDatabase extends AbstractUserDatabase { * @see org.apache.wiki.auth.user.UserDatabase#rename(String, String) */ @Override - public synchronized void rename( final String loginName, final String newName) throws NoSuchPrincipalException, DuplicateUserException, WikiSecurityException { + public synchronized void rename( final String loginName, final String newName) throws DuplicateUserException, WikiSecurityException { if( c_dom == null ) { log.fatal( "Could not rename profile '" + loginName + "'; database does not exist" ); throw new IllegalStateException( "FATAL: database does not exist" ); @@ -416,7 +418,7 @@ public class XMLUserDatabase extends AbstractUserDatabase { } } - // Save the attributes as as Base64 string + // Save the attributes as Base64 string if( profile.getAttributes().size() > 0 ) { try { final String encodedAttributes = Serializer.serializeToBase64( profile.getAttributes() ); @@ -443,8 +445,8 @@ public class XMLUserDatabase extends AbstractUserDatabase { * Private method that returns the first {@link UserProfile}matching a <user> element's supplied attribute. This method will also * set the UID if it has not yet been set. * - * @param matchAttribute - * @param index + * @param matchAttribute matching attribute + * @param index value to match * @return the profile, or <code>null</code> if not found */ private UserProfile findByAttribute( final String matchAttribute, String index ) { @@ -458,8 +460,8 @@ public class XMLUserDatabase extends AbstractUserDatabase { return null; } - // check if we have to do a case insensitive compare - boolean caseSensitiveCompare = !matchAttribute.equals( EMAIL ); + // check if we have to do a case-insensitive compare + final boolean caseSensitiveCompare = !matchAttribute.equals( EMAIL ); for( int i = 0; i < users.getLength(); i++ ) { final Element user = (Element) users.item( i ); @@ -489,13 +491,13 @@ public class XMLUserDatabase extends AbstractUserDatabase { // Is the profile locked? final String lockExpiry = user.getAttribute( LOCK_EXPIRY ); - if( lockExpiry == null || lockExpiry.isEmpty() ) { + if( StringUtils.isEmpty( lockExpiry ) || lockExpiry.isEmpty() ) { profile.setLockExpiry( null ); } else { profile.setLockExpiry( new Date( Long.parseLong( lockExpiry ) ) ); } - // Extract all of the user's attributes (should only be one attributes tag, but you never know!) + // Extract all the user's attributes (should only be one attributes tag, but you never know!) final NodeList attributes = user.getElementsByTagName( ATTRIBUTES_TAG ); for( int j = 0; j < attributes.getLength(); j++ ) { final Element attribute = ( Element )attributes.item( j ); @@ -515,7 +517,7 @@ public class XMLUserDatabase extends AbstractUserDatabase { } /** - * Extracts all of the text nodes that are immediate children of an Element. + * Extracts all the text nodes that are immediate children of an Element. * * @param element the base element * @return the text nodes that are immediate children of the base element, concatenated together @@ -537,8 +539,8 @@ public class XMLUserDatabase extends AbstractUserDatabase { /** * Tries to parse a date using the default format - then, for backwards compatibility reasons, tries the platform default. * - * @param profile - * @param date + * @param profile profile associated to the date. + * @param date date to be parsed. * @return A parsed date, or null, if both parse attempts fail. */ private Date parseDate( final UserProfile profile, final String date ) { @@ -571,7 +573,7 @@ public class XMLUserDatabase extends AbstractUserDatabase { // Sanitize UID (and generate a new one if one does not exist) String uid = user.getAttribute( UID ).trim(); - if( uid == null || uid.isEmpty() || "-1".equals( uid ) ) { + if( StringUtils.isEmpty( uid ) || "-1".equals( uid ) ) { uid = String.valueOf( generateUid( this ) ); user.setAttribute( UID, uid ); } diff --git a/jspwiki-main/src/test/java/org/apache/wiki/auth/user/XMLUserDatabaseTest.java b/jspwiki-main/src/test/java/org/apache/wiki/auth/user/XMLUserDatabaseTest.java index 6cd846a..f685f65 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/auth/user/XMLUserDatabaseTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/auth/user/XMLUserDatabaseTest.java @@ -40,9 +40,6 @@ public class XMLUserDatabaseTest { private XMLUserDatabase m_db; - /** - * - */ @BeforeEach public void setUp() throws Exception { final Properties props = TestEngine.getTestProperties(); @@ -58,7 +55,7 @@ public class XMLUserDatabaseTest { final int oldUserCount = m_db.getWikiNames().length; // Create a new user with random name - final String loginName = "TestUser" + String.valueOf( System.currentTimeMillis() ); + final String loginName = "TestUser" + System.currentTimeMillis(); UserProfile profile = m_db.newProfile(); profile.setEmail( "[email protected]" ); profile.setLoginName( loginName ); @@ -227,9 +224,9 @@ public class XMLUserDatabaseTest { public void testGetWikiNames() throws WikiSecurityException { // There are 8 test users in the database final Principal[] p = m_db.getWikiNames(); - Assertions.assertEquals( 8, p.length ); + Assertions.assertEquals( 7, p.length ); Assertions.assertTrue( ArrayUtils.contains( p, new WikiPrincipal( "JanneJalkanen", WikiPrincipal.WIKI_NAME ) ) ); - Assertions.assertTrue( ArrayUtils.contains( p, new WikiPrincipal( "", WikiPrincipal.WIKI_NAME ) ) ); + Assertions.assertFalse( ArrayUtils.contains( p, new WikiPrincipal( "", WikiPrincipal.WIKI_NAME ) ) ); Assertions.assertTrue( ArrayUtils.contains( p, new WikiPrincipal( "Administrator", WikiPrincipal.WIKI_NAME ) ) ); Assertions.assertTrue( ArrayUtils.contains( p, new WikiPrincipal( Users.ALICE, WikiPrincipal.WIKI_NAME ) ) ); Assertions.assertTrue( ArrayUtils.contains( p, new WikiPrincipal( Users.BOB, WikiPrincipal.WIKI_NAME ) ) ); @@ -271,7 +268,7 @@ public class XMLUserDatabaseTest { // The old user shouldn't be found try { - profile = m_db.findByLoginName( "olduser" ); + m_db.findByLoginName( "olduser" ); Assertions.fail( "Old user was found, but it shouldn't have been." ); } catch( final NoSuchPrincipalException e ) { // Cool, it's gone @@ -303,10 +300,8 @@ public class XMLUserDatabaseTest { // Make sure we can find it by uid final String uid = profile.getUid(); Assertions.assertNotNull( m_db.findByUid( uid ) ); - } catch( final NoSuchPrincipalException e ) { - Assertions.fail(); } catch( final WikiSecurityException e ) { - Assertions.fail(); + Assertions.fail( e.getMessage() ); } }
