diff --git a/ChangeLog.md b/ChangeLog.md index 9c0758815b..f82218fb27 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -35,6 +35,7 @@ under the License. * [JSPWIKI-1263](https://issues.apache.org/jira/browse/JSPWIKI-1263): the `Admin` group can no longer be removed, so the wiki cannot be left without administrators * [JSPWIKI-529](https://issues.apache.org/jira/browse/JSPWIKI-529): markup handling defect when passing css properties containing a percent symbol * Fix for [JSPWIKI-1130](https://issues.apache.org/jira/browse/JSPWIKI-1130): the preview of the editor no longer runs the scripts of the page, which could replace the whole editor page ([PR#560](https://github.com/apache/jspwiki/pull/560)) +* Fix for [JSPWIKI-1323](https://issues.apache.org/jira/browse/JSPWIKI-1323): a `JSPWikiUserPrefs` cookie that is not a JSON object is ignored, so the page keeps its stylesheets and scripts ([PR#577](https://github.com/apache/jspwiki/pull/577)) * Fix for [JSPWIKI-1324](https://issues.apache.org/jira/browse/JSPWIKI-1324): the time zone and time format lists of the user preferences are never wider than their form, so they stay within a narrow screen ([PR#578](https://github.com/apache/jspwiki/pull/578)) * Fix for [JSPWIKI-1354](https://issues.apache.org/jira/browse/JSPWIKI-1354): a test of the search no longer fails when the index has not got to all the pages ([PR#585](https://github.com/apache/jspwiki/pull/585)) diff --git a/jspwiki-main/src/main/java/org/apache/wiki/preferences/Preferences.java b/jspwiki-main/src/main/java/org/apache/wiki/preferences/Preferences.java index c3b99fb254..d6d4bc2821 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/preferences/Preferences.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/preferences/Preferences.java @@ -19,6 +19,7 @@ Licensed to the Apache Software Foundation (ASF) under one package org.apache.wiki.preferences; import com.google.gson.Gson; +import com.google.gson.JsonParseException; import org.apache.commons.lang3.LocaleUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; @@ -128,16 +129,33 @@ public static void reloadPreferences( final PageContext pageContext ) { /** - * Parses new-style preferences stored as JSON objects and stores them in the session. Everything in the cookie is stored. + * Parses new-style preferences stored as JSON objects and stores them in the session. Everything in the cookie is stored. The cookie + * comes from the client, so one that cannot be decoded or is not a JSON object is ignored and the default preferences stay. * * @param request * @param prefs The default hashmap of preferences */ - static void parseJSONPreferences( final HttpServletRequest request, final Preferences prefs ) { - final String prefVal = TextUtil.urlDecodeUTF8( HttpUtil.retrieveCookieValue( request, COOKIE_USER_PREFS_NAME ) ); + static void parseJSONPreferences( final HttpServletRequest request, final Preferences prefs ) { + final String prefVal; + try { + prefVal = TextUtil.urlDecodeUTF8( HttpUtil.retrieveCookieValue( request, COOKIE_USER_PREFS_NAME ) ); + } catch( final IllegalArgumentException e ) { + LOG.warn( "Ignoring the {} cookie, it cannot be decoded: {}", COOKIE_USER_PREFS_NAME, e.getMessage() ); + return; + } if( prefVal != null ) { // Convert prefVal JSON to a generic hashmap - @SuppressWarnings( "unchecked" ) final Map< String, String > map = new Gson().fromJson( prefVal, Map.class ); + final Map< String, String > map; + try { + @SuppressWarnings( "unchecked" ) final Map< String, String > parsed = new Gson().fromJson( prefVal, Map.class ); + map = parsed; + } catch( final JsonParseException e ) { + LOG.warn( "Ignoring the {} cookie, its value is not a JSON object: {}", COOKIE_USER_PREFS_NAME, e.getMessage() ); + return; + } + if( map == null ) { // an empty value or the JSON literal null + return; + } for( String key : map.keySet() ) { key = TextUtil.replaceEntities( key ); // Sometimes this is not a String as it comes from the Cookie set by Javascript diff --git a/jspwiki-main/src/test/java/org/apache/wiki/preferences/PreferencesTest.java b/jspwiki-main/src/test/java/org/apache/wiki/preferences/PreferencesTest.java index 6727f26210..c9871f17e7 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/preferences/PreferencesTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/preferences/PreferencesTest.java @@ -27,8 +27,63 @@ Licensed to the Apache Software Foundation (ASF) under one import java.net.URLEncoder; import java.nio.charset.StandardCharsets; +public class PreferencesTest { -class PreferencesTest { + private static Preferences parse( final String cookieValue ) { + final HttpServletRequest request = Mockito.mock( HttpServletRequest.class ); + if( cookieValue != null ) { + Mockito.when( request.getCookies() ).thenReturn( new Cookie[] { new Cookie( Preferences.COOKIE_USER_PREFS_NAME, cookieValue ) } ); + } + final Preferences prefs = new Preferences(); + prefs.put( "SkinName", "PlainVanilla" ); + Preferences.parseJSONPreferences( request, prefs ); + return prefs; + } + + /** https://issues.apache.org/jira/browse/JSPWIKI-1323 */ + @Test + public void cookieOverridesTheDefaults() { + final Preferences prefs = parse( "%7B%22SkinName%22%3A%22Smart%22%2C%22Layout%22%3A%22fixed%22%7D" ); + Assertions.assertEquals( "Smart", prefs.get( "SkinName" ) ); + Assertions.assertEquals( "fixed", prefs.get( "Layout" ) ); + } + + /** https://issues.apache.org/jira/browse/JSPWIKI-1323 */ + @Test + public void missingCookieKeepsTheDefaults() { + final Preferences prefs = parse( null ); + Assertions.assertEquals( "PlainVanilla", prefs.get( "SkinName" ) ); + Assertions.assertEquals( 1, prefs.size() ); + } + + /** https://issues.apache.org/jira/browse/JSPWIKI-1323 */ + @Test + public void cookieThatIsNotAJsonObjectIsIgnored() { + for( final String value : new String[] { "%7Bnot-json", "%5B1%2C2%5D", "123", "text", "%22text%22", "null", "true", "%7B", "%7B%22a%22%3A" } ) { + final Preferences prefs = Assertions.assertDoesNotThrow( () -> parse( value ), value ); + Assertions.assertEquals( "PlainVanilla", prefs.get( "SkinName" ), value ); + Assertions.assertEquals( 1, prefs.size(), value ); + } + } + + /** https://issues.apache.org/jira/browse/JSPWIKI-1323 */ + @Test + public void cookieWithBrokenPercentEncodingIsIgnored() { + for( final String value : new String[] { "%zz", "%7B%22a%22%3A%22b%22%7D%", "%E0%A4%A" } ) { + final Preferences prefs = Assertions.assertDoesNotThrow( () -> parse( value ), value ); + Assertions.assertEquals( "PlainVanilla", prefs.get( "SkinName" ), value ); + } + } + + /** https://issues.apache.org/jira/browse/JSPWIKI-1323 */ + @Test + public void valuesThatAreNotStringsAreKeptAsText() { + final Preferences prefs = parse( "%7B%22Sidebar%22%3Anull%2C%22Layout%22%3A%5B1%5D%2C%22Flag%22%3Atrue%7D" ); + Assertions.assertEquals( "PlainVanilla", prefs.get( "SkinName" ) ); + Assertions.assertFalse( prefs.containsKey( "Sidebar" ) ); + Assertions.assertEquals( "[1.0]", prefs.get( "Layout" ) ); + Assertions.assertEquals( "true", prefs.get( "Flag" ) ); + } private HttpServletRequest requestWithCookie( final String json ) { final HttpServletRequest request = Mockito.mock( HttpServletRequest.class );