Merge branch 'bug6088' into main

2026-09-15 10:18:45 +00:00 · 2025-01-26 00:14:48 +01:00
parent 4dcf06ccf5 934688f96c
commit da1e576905
16 changed files with 156 additions and 72 deletions
@@ -18,6 +18,26 @@ public abstract class AbstractMapTokenizer<P extends Place> implements PlaceToke

    @Override
    public P getPlace(String token) {
+        /*
+         * See bug 6088; the token delivered here as parameter has undergone encoding/decoding using URL.encodeURI; this
+         * leaves encoded characters with special meaning in the URL untouched, such as '/', '&' or '?', meaning that
+         * their encoding such as %2F for the '/' character will remain unchanged in the token and will not be replaced
+         * by '/'. Other characters, such as the '%' character, encoded as '%25', will be decoded.
+         * 
+         * But we do need to be able to encode special characters in tokens, e.g., for regatta names which may contain
+         * any UTF character. The problem, though, is that what we receive here as the "token" parameter is already a
+         * "projection" that cannot be inverted to the original string. For example, when we see "%2F" as the "token"
+         * value, we cannot tell whether the original URL hash/fragment was "%252F" or just "%2F" as the latter would
+         * have been returned unchanged.
+         * 
+         * Alternatively, we could look at Window.Location.getHash() and silently assume that that would be what was
+         * decoded and then passed to this method; interestingly, the call to here from SwitchingEntryPoint.onModuleLoad()
+         * passes an non-decoded hash/fragment taken straight from Window.Location.getHash().
+         * 
+         * Yet alternatively, we could find an encoding of values in tokens that is guaranteed to never produce the
+         * offending characters such as '%', '/' or the like. As an extreme case, we could use a Base64 encoding
+         * of all values and then run a Base64 decoding to obtain the original token again.
+         */
        return getPlaceFromParameters(toParameterMap(token));
    }

@@ -32,7 +52,6 @@ public abstract class AbstractMapTokenizer<P extends Place> implements PlaceToke

    private String toToken(Map<String, Set<String>> parameters) {
        StringBuilder stringBuilder = new StringBuilder();
-
        for (Map.Entry<String, Set<String>> entry : parameters.entrySet()) {
            if (entry.getValue() == null || entry.getValue().isEmpty()) {
                continue;
@@ -41,31 +60,34 @@ public abstract class AbstractMapTokenizer<P extends Place> implements PlaceToke
                if (stringBuilder.length() > 0) {
                    stringBuilder.append("&");
                }
-                stringBuilder.append(entry.getKey() + "=" + val);
+                stringBuilder.append(entry.getKey() + "=" + val); // see bug6088; values must contain only URL-compliant characters, so any encoding must be done by the caller
            }
        }
-
        return stringBuilder.toString();
    }

    private Map<String, Set<String>> toParameterMap(String placeParametersAsToken) {
-        List<String> list = Arrays.asList(placeParametersAsToken.split("&"));
-
+        final Map<String, Set<String>> result;
+        final List<String> list = Arrays.asList(placeParametersAsToken.split("&"));
        if (list == null || list.size() < 1) {
            logger.warning("Token empty, no-op");
-            Collections.emptyMap();
-        }
-
-        Map<String, Set<String>> result = new HashMap<>();
-        for (String listItem : list) {
-            String[] nvPair = listItem.split("=");
-            if (nvPair == null || nvPair.length != 2) {
-                logger.warning("Invalid parameters");
-                continue;
+            result = Collections.emptyMap();
+        } else {
+            result = new HashMap<>();
+            for (String listItem : list) {
+                final int indexOfEquals = listItem.indexOf('='); // don't use "split" because value may contain '=' characters
+                final String key;
+                final String value;
+                if (indexOfEquals < 0) {
+                    key = listItem;
+                    value = null;
+                } else {
+                    key = listItem.substring(0, indexOfEquals);
+                    value = listItem.substring(indexOfEquals+1);
+                }
+                Util.addToValueSet(result, key, value); // see bug6088; values are expected to travel safely through URLs; encoding is the caller's responsibility
            }
-            Util.addToValueSet(result, nvPair[0], nvPair[1]);
        }
        return result;
    }
-
 }
@@ -7,39 +7,44 @@ import com.sap.sse.gwt.client.Notification.NotificationType;

 public class NavigatorUtil {
    /**
-     * Note that this method will check whether the browser does support the copyToClipboard feature first 
-     * and will display a warning notification if it does not, to prevent JS error messages in the console.
-     * Please use {@link clientHasNavigatorCopyToClipboardSupport} to check for browser support, 
-     * if case specific handling is required.
-     * On a successful copy an info notification is displayed.
-     * @param String text: Text to share.
+     * Note that this method will check whether the browser does support the copyToClipboard feature first and will
+     * display a warning notification if it does not, to prevent JS error messages in the console. Please use
+     * {@link clientHasNavigatorCopyToClipboardSupport} to check for browser support, if case specific handling is
+     * required. On a successful copy an info notification is displayed.
+     * 
+     * @param String
+     *            text: Text to share.
     */
    public static void copyToClipboard(String text) {
-        if(nativeClientHasNavigatorCopyToClipboardSupport()) {
+        if (nativeClientHasNavigatorCopyToClipboardSupport()) {
            nativeCopyToClipboard(text);
            Notification.notify(StringMessages.INSTANCE.sharingLinkCopied(), NotificationType.INFO);
-        }else {
+        } else {
            GWT.log("This browser does not support copying to clipboard");
-            Notification.notify(StringMessages.INSTANCE.browserDoesNotSupportCopyToClipboard(), NotificationType.WARNING);
+            Notification.notify(StringMessages.INSTANCE.browserDoesNotSupportCopyToClipboard(),
+                    NotificationType.WARNING);
        }
    }
    
    
    /**
-     * Note that this method will check whether the browser does support the sharing feature first 
-     * and will display a warning notification if it does not, to prevent JS error messages in the console.
-     * Please use {@link clientHasNavigatorShareSupport} to check for browser support, if case specific handling is required.
-     * @param String url: URL to share.
-     * @param String text: Optional text to share.
+     * Note that this method will check whether the browser does support the sharing feature first and will display a
+     * warning notification if it does not, to prevent JS error messages in the console. Please use
+     * {@link clientHasNavigatorShareSupport} to check for browser support, if case specific handling is required.
+     * 
+     * @param String
+     *            url: URL to share.
+     * @param String
+     *            text: Optional text to share.
     */
    public static void shareUrl(String url, String text) {
-        if(nativeClientHasNavigatorShareSupport()) {
-            if(text != null) {
+        if (nativeClientHasNavigatorShareSupport()) {
+            if (text != null) {
                nativeShareUrlAndText(url, text);
-            }else {
+            } else {
                nativeShareUrl(url);
            }
-        }else {
+        } else {
            GWT.log("This browser does not support native sharing");
            Notification.notify(StringMessages.INSTANCE.browserDoesNotSupportNativeSharing(), NotificationType.WARNING);
        }
@@ -1,6 +1,7 @@
 package com.sap.sailing.gwt.home.shared.places.event;

 import java.util.UUID;
+import java.util.logging.Logger;

 import com.google.gwt.activity.shared.Activity;
 import com.sap.sailing.gwt.home.communication.event.GetEventViewAction;
@@ -21,7 +22,8 @@ import com.sap.sse.gwt.client.mvp.AbstractActivityProxy;
 import com.sap.sse.gwt.client.mvp.ClientFactory;

 public abstract class AbstractEventActivityProxy<C extends ClientFactory & ClientFactoryWithDispatch> extends AbstractActivityProxy implements ProvidesNavigationPath {
-    
+    private static final Logger logger = Logger.getLogger(AbstractEventActivityProxy.class.getName());
+
    private final C clientFactory;
    private AbstractEventPlace place;
    private NavigationPathDisplay navigationPathDisplay;
@@ -55,7 +57,7 @@ public abstract class AbstractEventActivityProxy<C extends ClientFactory & Clien
    }
    
    /**
-     * This method is called after loading the event an verifying the {@link AbstractEventPlace}.
+     * This method is called after loading the event and verifying the {@link AbstractEventPlace}.
     * Subclasses can start an {@link Activity} based on the given {@link AbstractEventPlace}.
     * 
     * @param clientFactory the {@link ClientFactoryWithDispatch} needed in {@link Activity}
@@ -96,6 +98,7 @@ public abstract class AbstractEventActivityProxy<C extends ClientFactory & Clien
                // Regatta ID unknown but unnecessary ...
                place.getCtx().withRegattaId(null);
            } else if (event.isMultiRegatta() && !regattaKnown) {
+                logger.warning("Regatta ID "+place.getCtx().getRegattaId()+" is unknown; displaying event instead");
                return new MultiregattaRegattasPlace(contextWithoutRegatta);
            }
        }
@@ -3,15 +3,20 @@ package com.sap.sailing.gwt.home.shared.places.event;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
+import java.util.logging.Logger;

+import com.google.gwt.http.client.URL;
 import com.google.gwt.place.shared.Place;
 import com.sap.sailing.gwt.common.client.AbstractMapTokenizer;
 import com.sap.sailing.gwt.home.shared.app.HasLocationTitle;
 import com.sap.sailing.gwt.ui.client.StringMessages;
+import com.sap.sse.common.Base64Utils;
 import com.sap.sse.common.Util;
 import com.sap.sse.gwt.shared.ClientConfiguration;

 public abstract class AbstractEventPlace extends Place implements HasLocationTitle {
+    private static final Logger logger = Logger.getLogger(AbstractEventPlace.class.getName());
+
    private final EventContext ctx;

    protected AbstractEventPlace(EventContext ctx) {
@@ -46,12 +51,29 @@ public abstract class AbstractEventPlace extends Place implements HasLocationTit
    }

    public static abstract class Tokenizer<PLACE extends AbstractEventPlace> extends AbstractMapTokenizer<PLACE> {
+        /**
+         * See also {@code TokenizedHomePlaceUrlBuilder.EVENT_ID_PARAM}
+         */
        private final static String PARAM_EVENTID = "eventId";
+
+        /**
+         * See also {@code TokenizedHomePlaceUrlBuilder.REGATTA_ID_PARAM}; expected to be a Base64 string that has been URL-encoded,
+         * so that the optional trailing '=' characters are properly encoded as %3D, for example.
+         */
        private final static String PARAM_REGATTAID = "regattaId";

        protected PLACE getPlaceFromParameters(Map<String, Set<String>> parameters) {
+            final String encodedRegattaId = extractSingleParameter(parameters, PARAM_REGATTAID);
+            String decodedRegattaId;
+            try {
+                decodedRegattaId = encodedRegattaId==null?null:new String(Base64Utils.fromBase64(encodedRegattaId));
+            } catch (Throwable e) {
+                logger.warning("Error trying to decode regatta ID "+encodedRegattaId+"; trying to use URL decoding to obtain regatta name");
+                decodedRegattaId = URL.decodeQueryString(encodedRegattaId);
+            }
+            // see bug 6088: regatta names may contain any UTF character and therefore need encoding
            return getRealPlace(new EventContext().withId(extractSingleParameter(parameters, PARAM_EVENTID))
-                    .withRegattaId(extractSingleParameter(parameters, PARAM_REGATTAID)), parameters);
+                    .withRegattaId(decodedRegattaId), parameters);
        }

        private String extractSingleParameter(Map<String, Set<String>> parameters, String key) {
@@ -63,9 +85,9 @@ public abstract class AbstractEventPlace extends Place implements HasLocationTit
            Map<String, Set<String>> parameters = new HashMap<>();
            EventContext context = place.getCtx();
            Util.addToValueSet(parameters, PARAM_EVENTID, context.getEventId());
-            String regattaId = context.getRegattaId();
+            String regattaId = context.getRegattaId(); // bug 6088: we assume that a regatta id/name can contain any UTF character and therefore needs encoding
            if (regattaId != null && !regattaId.isEmpty()) {
-                Util.addToValueSet(parameters, PARAM_REGATTAID, context.getRegattaId());
+                Util.addToValueSet(parameters, PARAM_REGATTAID, Base64Utils.toBase64(context.getRegattaId().getBytes()));
            }
            return parameters;
        }
@@ -53,13 +53,13 @@ public class EventContext implements ShareablePlaceContext {

    @Override
    public String getContextAsPathParameters() {
-        if(eventId != null) {
+        if (eventId != null) {
            String path = "/events/" + eventId;
-            if(regattaId != null) {
-                path += "/regattas/" +  URLEncoder.encode(regattaId);
+            if (regattaId != null) {
+                path += "/regattas/" + URLEncoder.encodeQueryString(regattaId);
            }
            return path;
-        }else {
+        } else {
            return null;
        }
    }
@@ -480,9 +480,9 @@ public class LeaderboardGroupConfigPanel extends AbstractRegattaPanel
            @Override
            public SafeHtml getValue(LeaderboardGroupDTO group) {
                String debugParam = Window.Location.getParameter("gwt.codesvr");
-                String link = URLEncoder.encode("/gwt/Spectator.html?leaderboardGroupId=" + group.getId()+"&showRaceDetails=true&"
+                String link = "/gwt/Spectator.html?leaderboardGroupId=" + group.getId()+"&showRaceDetails=true&"
                        + RaceBoardPerspectiveOwnSettings.PARAM_CAN_REPLAY_DURING_LIVE_RACES + "=true"
-                        + (debugParam != null && !debugParam.isEmpty() ? "&gwt.codesvr=" + debugParam : ""));
+                        + (debugParam != null && !debugParam.isEmpty() ? "&gwt.codesvr=" + URLEncoder.encodeQueryString(debugParam) : "");
                return ANCHORTEMPLATE.cell(UriUtils.fromString(link), group.getName());
            }
        };
@@ -362,10 +362,10 @@ public class LeaderboardGroupOverviewPanel extends FormPanel {
            public SafeHtml getValue(StrippedLeaderboardDTO leaderboard) {
                LeaderboardGroupDTO selectedGroup = groupsSelectionModel.getSelectedObject();
                String debugParam = Window.Location.getParameter("gwt.codesvr");
-                String link = URLEncoder.encode("/gwt/Leaderboard.html?name=" + leaderboard.getName()
+                String link = "/gwt/Leaderboard.html?name=" + URLEncoder.encodeQueryString(leaderboard.getName())
                        + (showRaceDetails ? "&showRaceDetails=true" : "") + "&root=overview" + "&leaderboardGroupId="
                        + selectedGroup.getId().toString()
-                        + (debugParam != null && !debugParam.isEmpty() ? "&gwt.codesvr=" + debugParam : ""));
+                        + (debugParam != null && !debugParam.isEmpty() ? "&gwt.codesvr=" + URLEncoder.encodeQueryString(debugParam) : "");
                return ANCHORTEMPLATE.anchor(UriUtils.fromString(link), leaderboard.getName());
            }
        };
@@ -455,11 +455,11 @@ public class LeaderboardGroupOverviewPanel extends FormPanel {
                        StrippedLeaderboardDTO selectedLeaderboard = leaderboardsSelectionModel.getSelectedObject();
                        RegattaNameAndRaceName raceId = (RegattaNameAndRaceName) race.getRaceIdentifier(fleet);
                        String debugParam = Window.Location.getParameter("gwt.codesvr");
-                        String link = URLEncoder.encode(RaceBoardPanel.RACEBOARD_PATH+"?leaderboardName="
-                                + selectedLeaderboard.getName() + "&raceName=" + raceId.getRaceName() + "&regattaName="
-                                + raceId.getRegattaName() + "&leaderboardGroupId=" + selectedGroup.getId().toString()
+                        String link = RaceBoardPanel.RACEBOARD_PATH+"?leaderboardName="
+                                + URLEncoder.encodeQueryString(selectedLeaderboard.getName()) + "&raceName=" + URLEncoder.encodeQueryString(raceId.getRaceName()) + "&regattaName="
+                                + URLEncoder.encodeQueryString(raceId.getRegattaName()) + "&leaderboardGroupId=" + selectedGroup.getId().toString()
                                + "&root=overview"
-                                + (debugParam != null && !debugParam.isEmpty() ? "&gwt.codesvr=" + debugParam : ""));
+                                + (debugParam != null && !debugParam.isEmpty() ? "&gwt.codesvr=" + URLEncoder.encodeQueryString(debugParam) : "");
                        name = ANCHORTEMPLATE.anchor(UriUtils.fromString(link), raceDisplayName);
                    } else {
                        name = new SafeHtmlBuilder().appendHtmlConstant(raceDisplayName).toSafeHtml();
@@ -11,6 +11,6 @@
        <meta http-equiv="refresh" content="0; url=${redirect_url}"> ${disable_redirect_end}
 	</head>
 	<body>
-	    <div>In case the redirect fails, please  <a href="${redirect_url_fallback}">click here</a>.</div>
+	    <div>In case the redirect fails, please <a href="${redirect_url_fallback}">click here</a>.</div>
 	</body>
 </html>
@@ -58,7 +58,7 @@ public class HomeSharingResource extends AbstractSailingServerResource {
    @Path("/events/{eventId}/regattas/{regattaId}")
    @Produces("text/html")
    public String getSharedRegatta(@PathParam("eventId") String eventId, @PathParam("regattaId") String regattaId,
-            @HeaderParam("user-agent") String userAgent) {
+            @HeaderParam("user-agent") String userAgent) throws UnsupportedEncodingException {
        RacingEventService eventService = getService();
        SecurityService securityService = getSecurityService();
        UUID uuid = UUID.fromString(eventId);
@@ -130,11 +130,14 @@ public class HomeSharingUtils {
    public static Map<String, String> createReplacementMap(String title, String description,
            String imageUrl, String placeUrl, String userAgent) {
        final Map<String, String> replacementMap = new HashMap<String, String>();
-        String disabledStart = "";
-        String disabledEnd = "";
-        if(userAgent != null && userAgent.contains("facebookexternalhit")) {
+        final String disabledStart;
+        final String disabledEnd;
+        if (userAgent != null && userAgent.contains("facebookexternalhit")) {
            disabledStart = HTML_COMMENT_START;
            disabledEnd = HTML_COMMENT_END;
+        } else {
+            disabledStart = "";
+            disabledEnd = "";
        }
        replacementMap.put(REPLACEMENT_KEY_TITLE, title);
        replacementMap.put(REPLACEMENT_KEY_DESCRIPTION, description);
@@ -6,6 +6,8 @@ import java.net.URL;

 import javax.ws.rs.core.UriInfo;

+import com.sap.sse.common.Base64Utils;
+
 public class TokenizedHomePlaceUrlBuilder {
    private static final String EVENT_PATH = "/event";
    private static final String SERIES_PATH = "/series";
@@ -14,7 +16,15 @@ public class TokenizedHomePlaceUrlBuilder {
    private static final String HOME_HTML = "/Home.html";
    private static final String STARTOFPARAMS = "/:";
    private static final String STARTOFTOKENPATH = "#";
+
+    /**
+     * See also {@code AbstractEventPlace.Tokenizer.PARAM_EVENTID}
+     */
    private static final String EVENT_ID_PARAM = "eventId";
+    
+    /**
+     * See also {@code AbstractEventPlace.Tokenizer.PARAM_REGATTAID}
+     */
    private static final String REGATTA_ID_PARAM = "regattaId";
    private static final String LEADERBOARD_GROUP_ID_PARAM = "leaderboardGroupId";

@@ -49,7 +59,7 @@ public class TokenizedHomePlaceUrlBuilder {
        String url = new String(baseUrl.toString());
        url += STARTOFTOKENPATH + REGATTA_OVERVIEW_PATH;
        url += buildToken(eventId, EVENT_ID_PARAM);
-        url += buildToken(regattaId, REGATTA_ID_PARAM);
+        url += buildToken(Base64Utils.toBase64(regattaId.getBytes()), REGATTA_ID_PARAM);
        return url;
    }

@@ -4,7 +4,7 @@ import com.google.gwt.http.client.URL;

 /**
 * A very basic URL encoder that also works in a GWT environment where <code>java.net.URLEncoder</code> is not available.
- * It is based on {@link URL#encode(String)} and adds encoding for the backslash (\) character.
+ * It is based on {@link URL#encodeQueryString(String)}.
 * 
 * @author Axel Uhl (D043530)
 *
@@ -13,14 +13,8 @@ public class URLEncoder {
    
    private URLEncoder() { }

-    private final static char[] charsToReplace = { '\'', '(', ')', '"', '[', ']', '{', '}', '<', '>', '|', '+' };
-    
-    public static String encode(String url) {
-        String nearlyEncoded = URL.encode(url);
-        for (char c : charsToReplace) {
-            nearlyEncoded = nearlyEncoded.replaceAll("\\" + Character.toString(c), "%" + Integer.toHexString(c));
-        }
-        return nearlyEncoded;
+    public static String encodeQueryString(String url) {
+        return URL.encodeQueryString(url);
    }
    
 }
@@ -51,6 +51,12 @@ public class PermissionConverterTest {
        com.sap.sse.security.shared.WildcardPermission wp = new PermissionConverter().getWildcardPermission(new org.apache.shiro.authz.permission.WildcardPermission("LEADERBOARD:EDIT:KW2017 Laser Int.", /* case sensitive */ true));
        assertEquals("LEADERBOARD:EDIT:KW2017 Laser Int.", wp.toString());
    }
+
+    @Test
+    public void testGetWildcardPermissionWithMultipleObjectsInThirdPart() {
+        com.sap.sse.security.shared.WildcardPermission wp = new PermissionConverter().getWildcardPermission(new org.apache.shiro.authz.permission.WildcardPermission("LEADERBOARD:EDIT:KW2017 Laser Int.,KW2024 [ILCA7]", /* case sensitive */ true));
+        assertEquals("LEADERBOARD:EDIT:KW2017 Laser Int.,KW2024 [ILCA7]", wp.toString());
+    }
    
    @Test
    public void testMixedCaseWildcardPermissionWithDedicatedRealm() {
@@ -20,6 +20,7 @@ import org.osgi.framework.BundleContext;
 import org.osgi.util.tracker.ServiceTracker;

 import com.sap.sse.security.impl.Activator;
+import com.sap.sse.security.impl.PermissionConverter;
 import com.sap.sse.security.interfaces.AccessControlStore;
 import com.sap.sse.security.interfaces.UserStore;
 import com.sap.sse.security.shared.AccessControlListAnnotation;
@@ -169,7 +170,7 @@ public abstract class AbstractCompositeAuthorizingRealm extends AuthorizingRealm
    public boolean isPermitted(PrincipalCollection principals, Permission perm) {
        String username = (String) principals.getPrimaryPrincipal();
        final User user = getUserStore().getUserByName(username);
-        final WildcardPermission wildcardPermission = new WildcardPermission(perm.toString().replaceAll("\\[|\\]", ""));
+        final WildcardPermission wildcardPermission = new PermissionConverter().getWildcardPermission(perm);
        List<Set<String>> parts = wildcardPermission.getParts();
        final boolean result;
        if (parts.size() > 2 && !parts.get(2).isEmpty()) {
@@ -27,13 +27,11 @@ public class PermissionConverter {
    private final PermissionToObjectIdConverter poc = new PermissionToObjectIdConverter();
    
    public WildcardPermission getWildcardPermission(Permission permission) {
-        return new WildcardPermission(getAsString(permission));
+        final WildcardPermission result;
+        result = new WildcardPermission(permission.toString());
+        return result;
    }

-    private String getAsString(Permission permission) {
-        return permission.toString().replaceAll("\\[|\\]", ""); // FIXME for multi-subpart parts remove leading blanks after ,
-    }
-    
    /**
     * Splits the permission along ":" occurrences
     */
@@ -0,0 +1,20 @@
+package com.sap.sse.test;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+
+import org.junit.Test;
+
+public class URLEncoderTest {
+    @Test
+    public void testWithSlash() throws UnsupportedEncodingException {
+        assertEquals("a%2Fb", URLEncoder.encode("a/b", "UTF-8"));
+    }
+
+    @Test
+    public void testWithColon() throws UnsupportedEncodingException {
+        assertEquals("a%3Ab", URLEncoder.encode("a:b", "UTF-8"));
+    }
+}