Sling
  1. Sling
  2. SLING-2913

Issue in AbstractCreateOperation#deepGetOrCreateNode

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: Servlets Post 2.3.2
    • Component/s: Servlets
    • Labels:
      None

      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.

        Activity

          People

          • Assignee:
            Carsten Ziegeler
            Reporter:
            Antonio Sanso
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development