Skip to content

fix: mount and browse space drives whose display name contains a slash - EXO-89613 #2070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ahamdi merged 1 commit into from
Sep 3, 2026
Merged

fix: mount and browse space drives whose display name contains a slash - EXO-89613 #2070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 75 additions & 22 deletions ...src/main/java/org/exoplatform/documents/storage/jcr/webdav/plugin/PathCommandHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import javax.jcr.*;
Expand Down Expand Up @@ -95,6 +95,15 @@ public class PathCommandHandler {

public static final String PATHS_CONCAT_FORMAT = "%s/%s";

public static final String SEGMENT_CHAR_REPLACEMENT = "_";

/**
* Characters a name cannot carry into a WebDAV path segment: the two path
* separators, and the characters whose percent-encoded form is in
* {@code StrictHttpFirewall}'s blocklist.
*/
private static final Pattern UNSAFE_SEGMENT_CHARS = Pattern.compile("[/\\\\%;\\p{Cntrl}]");

protected static final Log LOG = ExoLogger.getLogger(PathCommandHandler.class);

private static final String WEBDAV_IDENTITY_JCR_PATH_CACHE_NAME = "webdav.identityJcrBasePath";
Expand Down Expand Up @@ -146,6 +155,23 @@ public void init() {
addMappingEventListener();
}

/**
* Last-resort guard keeping a drive name inside a single WebDAV path segment.
* The names fed to it — a Space pretty name, a username — are already URL-safe
* by construction (see {@link #getIdentitySegmentName(Identity)}); this only
* makes sure an identity store that yields something unexpected cannot emit a
* '%2F', which is rejected before the request reaches any handler and which,
* once decoded, would split the drive into two segments so that the identity
* id can no longer be read back from the path.
*
* @param segmentName drive name, may be null
* @return the name with every character unusable in a path segment replaced
* by {@link #SEGMENT_CHAR_REPLACEMENT}
*/
public static String toWebDavSegment(String segmentName) {
return UNSAFE_SEGMENT_CHARS.matcher(StringUtils.defaultString(segmentName)).replaceAll(SEGMENT_CHAR_REPLACEMENT);
}

@SneakyThrows
@Cacheable(WEBDAV_IDENTITY_JCR_PATH_CACHE_NAME)
public String getIdentityBaseJcrPath(String webDavPath) {
Expand Down Expand Up @@ -264,11 +290,13 @@ public String resolveToJcrPath(Session session, String webDavPath) throws WebDav
if (session.itemExists(legacyChildJcrPath)) {
Item existingItem = session.getItem(legacyChildJcrPath);
if (existingItem instanceof Node existingNode) {
String existingWebDavPath = getOrCreateWebDavPath(existingNode);
String decodedExistingWebDavPath = decodeUrlString(existingWebDavPath);
String decodedExistingWebDavPathPrefix = StringUtils.removeEnd(decodedExistingWebDavPath, "/") + "/"; // NOSONAR
if (webDavPath.startsWith(decodedExistingWebDavPathPrefix)
|| webDavPath.equals(decodedExistingWebDavPath)) {
// Compare the identity-relative parts only: the drive segment is
// addressed by its id, so a client may legitimately hold an older
// spelling of the drive name in the path it sends
String existingRelativePath = getIdentityRelativeDecodedWebDavPath(getOrCreateWebDavPath(existingNode));
if (StringUtils.isBlank(existingRelativePath)
|| StringUtils.equals(identityRelativeWebDavPath, existingRelativePath)
|| StringUtils.startsWith(identityRelativeWebDavPath, existingRelativePath + "/")) {
currentParentJcrPath = legacyChildJcrPath;
continue;
}
Expand Down Expand Up @@ -307,7 +335,7 @@ public String getOrCreateWebDavPath(Node node) {
return null;
}
String identityBaseJcrPath = getIdentityBaseJcrPath(identityId);
String identityRootWebDavPath = getIdentityRootWebDavPath(identityId, getIdentityDisplayName(identity));
String identityRootWebDavPath = getIdentityRootWebDavPath(identityId, getIdentitySegmentName(identity));
return getOrCreateWebDavPath(String.valueOf(identityId),
identityBaseJcrPath,
identityRootWebDavPath,
Expand Down Expand Up @@ -486,6 +514,19 @@ private String getIdentityRelativeWebDavPath(String webDavPath) {
.collect(Collectors.joining("/"));
}

/**
* @param encodedWebDavPath a WebDAV path as stored/emitted, percent-encoded
* @return the same path without its drive segment, each remaining segment
* decoded — comparable with the decoded path a client sends
*/
private String getIdentityRelativeDecodedWebDavPath(String encodedWebDavPath) {
return Arrays.stream(StringUtils.defaultString(encodedWebDavPath).split("/"))
.filter(StringUtils::isNotBlank)
.skip(1)
.map(this::decodeUrlString)
.collect(Collectors.joining("/"));
}

private List<String> splitDecodedSegments(String relativeWebDavPath) {
return Arrays.stream(relativeWebDavPath.split("/"))
.filter(StringUtils::isNotBlank)
Expand Down Expand Up @@ -734,30 +775,42 @@ private Long getIdentityIdFromJcrPath(String jcrPath) {
}

private String getIdentityRootWebDavPath(long identityId) {
return getIdentityRootWebDavPath(identityId, getIdentityDisplayName(identityId));
return getIdentityRootWebDavPath(identityId, getIdentitySegmentName(identityManager.getIdentity(identityId)));
}

private String getIdentityRootWebDavPath(long identityId, String displayName) {
private String getIdentityRootWebDavPath(long identityId, String segmentName) {
return String.format("/%s%s%s%s",
encodeUrlString(displayName),
encodeUrlString(toWebDavSegment(segmentName)),
IDENTITY_ID_PREFIX,
identityId,
IDENTITY_ID_SUFFIX);
}

private String getIdentityDisplayName(long identityId) {
Identity identity = identityManager.getIdentity(identityId);
return Objects.requireNonNullElseGet(getIdentityDisplayName(identity), () -> String.valueOf(identityId));
}

private String getIdentityDisplayName(Identity identity) {
if (identity != null && identity.isSpace()) {
Space space = spaceService.getSpaceByPrettyName(identity.getRemoteId());
if (space != null) {
return space.getDisplayName();
}
/**
* Returns the name an identity contributes to its WebDAV drive segment:
* <ul>
* <li>a Space is addressed by its <b>pretty name</b>, the very name its drive
* is created under in JCR
* (<code>/groups/spaces/&lt;prettyName&gt;/Documents</code>): URL-safe by
* construction and frozen at creation, unlike the Space display name, which a
* rename can change and which may carry a '/';</li>
* <li>a personal drive keeps the user <b>full name</b>, which reads far better
* than a username when the drive is mounted, and which
* {@link #toWebDavSegment(String)} keeps inside a single path segment.</li>
* </ul>
*
* @param identity {@link Identity} of the drive owner, may be null
* @return the name the drive is addressed by, never null
*/
public static String getIdentitySegmentName(Identity identity) {
if (identity == null) {
return "";
}
if (identity.isSpace()) {
return StringUtils.defaultIfBlank(identity.getRemoteId(), identity.getId());
}
return identity == null ? null : identity.getProfile().getFullName();
String fullName = identity.getProfile() == null ? null : identity.getProfile().getFullName();
return StringUtils.firstNonBlank(fullName, identity.getRemoteId(), identity.getId(), "");
}

@SneakyThrows
Expand Down
39 changes: 22 additions & 17 deletions ...in/java/org/exoplatform/documents/storage/jcr/webdav/plugin/WebdavReadCommandHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import static org.exoplatform.documents.storage.jcr.webdav.plugin.PathCommandHandler.IDENTITY_PATHS_FORMAT;
import static org.exoplatform.documents.storage.jcr.webdav.plugin.PathCommandHandler.LOG;
import static org.exoplatform.documents.storage.jcr.webdav.plugin.PathCommandHandler.PROPERTY_NAMES;
import static org.exoplatform.documents.storage.jcr.webdav.plugin.PathCommandHandler.getIdentitySegmentName;
import static org.exoplatform.documents.storage.jcr.webdav.plugin.PathCommandHandler.toWebDavSegment;
import static org.exoplatform.documents.webdav.model.constant.PropertyConstants.CHECKEDIN;
import static org.exoplatform.documents.webdav.model.constant.PropertyConstants.CHECKEDOUT;
import static org.exoplatform.documents.webdav.model.constant.PropertyConstants.CHILDCOUNT;
Expand Down Expand Up @@ -190,7 +192,7 @@ public WebDavItem get(Session session, // NOSONAR
return get(getNode(session, pathCommandHandler.resolveToJcrPath(session, webDavPath)),
pathCommandHandler.getIdentityBaseJcrPath(webDavPath),
identity.getIdentityId(),
identity.getProfile().getFullName(),
getIdentitySegmentName(identity),
requestedPropertyNames,
requestPropertyNamesOnly,
depth,
Expand Down Expand Up @@ -261,7 +263,7 @@ public List<WebDavItem> getVersions(Session session, String webDavPath, Set<QNam
.map(version -> get(getVersionNode(version),
identityBaseJcrPath,
identity.getIdentityId(),
identity.getProfile().getFullName(),
getIdentitySegmentName(identity),
requestedPropertyNames,
false,
0,
Expand Down Expand Up @@ -365,7 +367,7 @@ private WebDavItem get(Node node,
return get(node,
pathCommandHandler.getIdentityBaseJcrPath(identityId),
identity.getIdentityId(),
identity.getProfile().getFullName(),
getIdentitySegmentName(identity),
requestedPropertyNames,
false,
0,
Expand All @@ -377,7 +379,7 @@ private WebDavItem get(Node node,
private WebDavItem get(Node node, // NOSONAR
String identityBaseJcrPath,
long identityId,
String displayName,
String segmentName,
Set<QName> requestedPropertyNames,
boolean requestPropertyNamesOnly,
int depth,
Expand All @@ -387,8 +389,8 @@ private WebDavItem get(Node node, // NOSONAR
}
WebDavItem result = new WebDavItem();
result.setFile(isFile(node));
String webDavPath = getMappedWebDavPath(node, identityBaseJcrPath, identityId, displayName);
String identityRootWebDavPath = getIdentityRootWebDavPath(identityId, displayName);
String webDavPath = getMappedWebDavPath(node, identityBaseJcrPath, identityId, segmentName);
String identityRootWebDavPath = getIdentityRootWebDavPath(identityId, segmentName);
String identifier = identityBaseUri;
if (!StringUtils.equals(webDavPath, identityRootWebDavPath)) {
identifier = identityBaseUri + webDavPath.substring(identityRootWebDavPath.length());
Expand All @@ -400,7 +402,7 @@ private WebDavItem get(Node node, // NOSONAR
node,
identityBaseJcrPath,
identityId,
displayName,
segmentName,
requestedPropertyNames,
requestPropertyNamesOnly,
depth,
Expand All @@ -418,7 +420,7 @@ private void addChildren(WebDavItem webDavItem, // NOSONAR
Node node,
String identityBaseJcrPath,
long identityId,
String displayName,
String segmentName,
Set<QName> requestedPropertyNames,
boolean requestPropertyNamesOnly,
int depth,
Expand All @@ -431,7 +433,7 @@ private void addChildren(WebDavItem webDavItem, // NOSONAR
.map(childNode -> get(childNode,
identityBaseJcrPath,
identityId,
displayName,
segmentName,
requestedPropertyNames,
requestPropertyNamesOnly,
depth - 1,
Expand Down Expand Up @@ -706,9 +708,12 @@ private WebDavItem getWebDavIdentityItem(Session session, // NOSONAR
displayName = getDisplayName(identityId);
}
WebDavItem identityWebDavItem = new WebDavItem();
// The drive is addressed by its segment name (Space pretty name / username)
// and only presented under its display name
String segmentName = getIdentitySegmentName(identityManager.getIdentity(identityId));
String identityBaseUri = String.format(IDENTITY_PATHS_FORMAT,
baseUri,
encodeUrlString(displayName),
encodeUrlString(toWebDavSegment(segmentName)),
IDENTITY_ID_PREFIX,
identityId,
IDENTITY_ID_SUFFIX);
Expand All @@ -723,7 +728,7 @@ private WebDavItem getWebDavIdentityItem(Session session, // NOSONAR
identityWebDavItem.setWebDavPath(getMappedWebDavPath(identityParentNode,
identityBaseJcrPath,
identityId,
displayName));
segmentName));
addProperties(identityWebDavItem,
identityParentNode,
requestedPropertyNames,
Expand All @@ -733,7 +738,7 @@ private WebDavItem getWebDavIdentityItem(Session session, // NOSONAR
identityParentNode,
identityBaseJcrPath,
identityId,
displayName,
segmentName,
requestedPropertyNames,
requestPropertyNamesOnly,
depth,
Expand Down Expand Up @@ -765,9 +770,9 @@ private void addProperties(WebDavItem result,
}
}

private String getIdentityRootWebDavPath(long identityId, String displayName) {
private String getIdentityRootWebDavPath(long identityId, String segmentName) {
return String.format("/%s%s%s%s",
encodeUrlString(displayName),
encodeUrlString(toWebDavSegment(segmentName)),
IDENTITY_ID_PREFIX,
identityId,
IDENTITY_ID_SUFFIX);
Expand All @@ -777,8 +782,8 @@ private String getIdentityRootWebDavPath(long identityId, String displayName) {
private String getMappedWebDavPath(Node node,
String identityBaseJcrPath,
long identityId,
String displayName) {
String identityRootWebDavPath = getIdentityRootWebDavPath(identityId, displayName);
String segmentName) {
String identityRootWebDavPath = getIdentityRootWebDavPath(identityId, segmentName);
if (StringUtils.equals(node.getPath(), identityBaseJcrPath)) {
return identityRootWebDavPath;
} else {
Expand All @@ -804,7 +809,7 @@ private String getIdentityBaseUri(String baseUri, long identityId) {
private String getIdentityBaseUri(String baseUri, Identity identity) {
return String.format(IDENTITY_PATHS_FORMAT,
baseUri,
encodeUrlString(identity.getProfile().getFullName()),
encodeUrlString(toWebDavSegment(getIdentitySegmentName(identity))),
IDENTITY_ID_PREFIX,
identity.getId(),
IDENTITY_ID_SUFFIX);
Expand Down
Loading
Loading