[SLING-2913] Issue in AbstractCreateOperation#deepGetOrCreateNode

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: Servlets Post 2.3.2
• Component/s: Servlets
• Assignee: Carsten Ziegeler
• Reporter: Antonio Sanso
• Created: 06/Jun/13 14:21
• Updated: 18/Jul/13 07:55
• Resolved: 09/Jul/13 07:22

Description

While updating the AbstractCreateOperation to use new CRUD support in POST servlet has been some lost in translation.

The previous code looked like

    protected Node deepGetOrCreateNode(Session session, String path,
            Map<String, RequestProperty> reqProperties, List<Modification> changes,
            VersioningConfiguration versioningConfiguration)
            throws RepositoryException {
        if (log.isDebugEnabled()) {
            log.debug("Deep-creating Node '{}'", path);
        }
        if (path == null || !path.startsWith("/")) {
            throw new IllegalArgumentException("path must be an absolute path.");
        }
        // get the starting node
        String startingNodePath = path;
        Node startingNode = null;
        while (startingNode == null) {
            if (startingNodePath.equals("/")) {
                startingNode = session.getRootNode();
            } else if (session.itemExists(startingNodePath)) {
                startingNode = (Node) session.getItem(startingNodePath);
                updateNodeType(session, startingNodePath, reqProperties, changes, versioningConfiguration);
                updateMixins(session, startingNodePath, reqProperties, changes, versioningConfiguration);
            } else {
                int pos = startingNodePath.lastIndexOf('/');
                if (pos > 0) {
                    startingNodePath = startingNodePath.substring(0, pos);
                } else {
                    startingNodePath = "/";
                }
            }
        }

while the updated is

protected Resource deepGetOrCreateNode(final ResourceResolver resolver,
                    final String path,
                    final Map<String, RequestProperty> reqProperties,
                    final List<Modification> changes,
                    final VersioningConfiguration versioningConfiguration)
    throws PersistenceException, RepositoryException {
        if (log.isDebugEnabled()) {
            log.debug("Deep-creating resource '{}'", path);
        }
        if (path == null || !path.startsWith("/")) {
            throw new IllegalArgumentException("path must be an absolute path.");
        }
        // get the starting resource
        String startingResourcePath = path;
        Resource startingResource = null;
        while (startingResource == null) {
            if (startingResourcePath.equals("/")) {
                startingResource = resolver.getResource("/");
            } else if (resolver.getResource(startingResourcePath) != null) {
                startingResource = resolver.getResource(startingResourcePath);
                updateNodeType(resolver, startingResourcePath, reqProperties, changes, versioningConfiguration);
                updateMixins(resolver, startingResourcePath, reqProperties, changes, versioningConfiguration);
            } else {
                int pos = startingResourcePath.lastIndexOf('/');
                if (pos > 0) {
                    startingResourcePath = startingResourcePath.substring(0, pos);
                } else {
                    startingResourcePath = "/";
                }
            }
        }

The main difference is in that the

startingNode = session.getRootNode(); could throw a RepositoryException e.g. if the session did not have enough permission while startingResource = resolver.getResource("/"); would just return null.

This might cause a theoretical infinite loop.