From 0a67663e34797d91a8c09d18e2ad99e41d3187fe Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Tue, 3 Nov 2026 21:09:17 +0100 Subject: [PATCH 1/2] JSPWIKI-1323 Ignore a preferences cookie that is not a JSON object The cookie is client data. A value that is not JSON, or not a JSON object, made Gson throw inside the page header, so the page was sent without its stylesheets and scripts, and the script that replaces the cookie never ran. Such a cookie is now ignored and the default preferences are used. --- .../apache/wiki/preferences/Preferences.java | 26 +++++- .../wiki/preferences/PreferencesTest.java | 86 +++++++++++++++++++ 2 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 jspwiki-main/src/test/java/org/apache/wiki/preferences/PreferencesTest.java 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 5e6b0fa290..c6249d2b83 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; @@ -123,16 +124,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 */ - private 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 new file mode 100644 index 0000000000..11443468ee --- /dev/null +++ b/jspwiki-main/src/test/java/org/apache/wiki/preferences/PreferencesTest.java @@ -0,0 +1,86 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + */ +package org.apache.wiki.preferences; + +import jakarta.servlet.http.Cookie; +import jakarta.servlet.http.HttpServletRequest; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + + +public 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" ) ); + } + +} From 8375c0255069f60e33374e4d0ee22472b9198e9b Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Tue, 3 Nov 2026 21:10:20 +0100 Subject: [PATCH 2/2] JSPWIKI-1323 Add the entry of the change to the ChangeLog --- ChangeLog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog.md b/ChangeLog.md index 6950e04176..555ff5f3c2 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -39,6 +39,7 @@ under the License. * Fix for [JSPWIKI-922](https://issues.apache.org/jira/browse/JSPWIKI-922): the `Content-Disposition` header of an attachment download carried a name with non-ASCII characters as a raw byte, which is not valid UTF-8; such names get an ASCII fallback and a `filename*` parameter with the UTF-8 name now ([PR#571](https://github.com/apache/jspwiki/pull/571)) * Fix for [JSPWIKI-501](https://issues.apache.org/jira/browse/JSPWIKI-501): wildcard and prefix searches that contain an underscore find the pages ([PR#543](https://github.com/apache/jspwiki/pull/543)) +* 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-1138](https://issues.apache.org/jira/browse/JSPWIKI-1138): Install.jsp echoed submitted values without escaping them; they are escaped now ([PR#323](https://github.com/apache/jspwiki/pull/323)) * Fix for [JSPWIKI-1164](https://issues.apache.org/jira/browse/JSPWIKI-1164): a page reached from a search result showed `<` as `<` when the query was highlighted ([PR#538](https://github.com/apache/jspwiki/pull/538)) * Fix for [JSPWIKI-919](https://issues.apache.org/jira/browse/JSPWIKI-919): `VersioningFileProvider#pageExists()` ignored the first version of a page that has no history, so a page dropped into the page directory was reported as missing when the page cache is off ([PR#539](https://github.com/apache/jspwiki/pull/539))