Details

      Description

      There are several minor issues in the mapping between JCR lock tokens and WebDAV lock tokens:

      1) WebDAV lock tokens are supposed to use URI syntax (such as opaquelocktoken: or urn:uuid

      2) The server currently computes lock tokens for session-scoped locks based on the node id; these are not valid JCR lock tokens though and cause exceptions when they are re-added when they appear in a Lock-Token header or an If header. This will likely cause requests to fail that use both types of locks (yes, maybe academic but should be fixed anyway)

      Proposal:

      a) Map lock tokens to oqaquelocktoken URIs, using a constant UUID plus a postfix encoding the original lock token
      b) Use a syntax that allows to distinguish between tokens for open-scoped locks or session-scoped locks, so that we do not try to add the latter type to the Session (alternatively, handle exceptions doing so gracefully)

      1. JCR-3209.diff
        11 kB
        Julian Reschke

        Issue Links

          Activity

          Hide
          angela added a comment -

          isn't 1) already covered by JCR-1352?

          regarding 2)

          > The server currently computes lock tokens for session-scoped locks based on the node id; these are not valid JCR lock tokens though

          this change was introduce since as of jcr 2.0 session scoped locks don't expose the token any more.
          if you have a proposal for another type of token that's fine.

          > and cause exceptions when they are re-added when they appear in a Lock-Token header or an If header.

          as far as i know this just causes a ugly warning in the log file (written by jackrabbit-core), since adding the token to
          session fails, which in this case was useless any way as they are session scoped. but if i remember correctly it otherwise
          used to work (see also JCR-2990... which i resolved wontfix due to lack of priority)

          > This will likely cause requests to fail that use both types of locks (yes, maybe academic but should be fixed anyway)

          can't follow you here.

          but anyway: it was definitely favorable if we can distinguish the two types of locks based on the token and
          could therefore omit adding it to the session/lockmanager.

          Show
          angela added a comment - isn't 1) already covered by JCR-1352 ? regarding 2) > The server currently computes lock tokens for session-scoped locks based on the node id; these are not valid JCR lock tokens though this change was introduce since as of jcr 2.0 session scoped locks don't expose the token any more. if you have a proposal for another type of token that's fine. > and cause exceptions when they are re-added when they appear in a Lock-Token header or an If header. as far as i know this just causes a ugly warning in the log file (written by jackrabbit-core), since adding the token to session fails, which in this case was useless any way as they are session scoped. but if i remember correctly it otherwise used to work (see also JCR-2990 ... which i resolved wontfix due to lack of priority) > This will likely cause requests to fail that use both types of locks (yes, maybe academic but should be fixed anyway) can't follow you here. but anyway: it was definitely favorable if we can distinguish the two types of locks based on the token and could therefore omit adding it to the session/lockmanager.
          Hide
          Julian Reschke added a comment -

          Angela, sorry for missing the two older JIRA issues Will clean this up.

          Regarding:

          "> This will likely cause requests to fail that use both types of locks (yes, maybe academic but should be fixed anyway)

          can't follow you here. "

          My reading of the code is that a RuntimeException is thrown and thus the request will not continue to run as it should (but I'll check that).

          Show
          Julian Reschke added a comment - Angela, sorry for missing the two older JIRA issues Will clean this up. Regarding: "> This will likely cause requests to fail that use both types of locks (yes, maybe academic but should be fixed anyway) can't follow you here. " My reading of the code is that a RuntimeException is thrown and thus the request will not continue to run as it should (but I'll check that).
          Hide
          Julian Reschke added a comment -

          Angela: indeed, the IllegalArgumentException is transformed into a LockException and then catched, logged, and ignored by o.a.j.c.SessionImpl,addLockToken(). Sorry for the confusion.

          Show
          Julian Reschke added a comment - Angela: indeed, the IllegalArgumentException is transformed into a LockException and then catched, logged, and ignored by o.a.j.c.SessionImpl,addLockToken(). Sorry for the confusion.
          Hide
          angela added a comment -

          no problem... i just happened to take a look at that the session-scoped locks some time ago
          and found myself struggling with the log-output, that turned out to be irrelevant for the
          problem i was trying to solve.

          Show
          angela added a comment - no problem... i just happened to take a look at that the session-scoped locks some time ago and found myself struggling with the log-output, that turned out to be irrelevant for the problem i was trying to solve.
          Hide
          Julian Reschke added a comment -

          Proposed patch; implements mapping of JCR lock tokens to valid DAV lock token URIs, isolates mapping in a single place.

          Show
          Julian Reschke added a comment - Proposed patch; implements mapping of JCR lock tokens to valid DAV lock token URIs, isolates mapping in a single place.
          Hide
          Julian Reschke added a comment -

          Observation: it appears that we never need the actual JCR lock token of a session scoped lock. That makes sense as we are keeping the JCR session around, right?

          Show
          Julian Reschke added a comment - Observation: it appears that we never need the actual JCR lock token of a session scoped lock. That makes sense as we are keeping the JCR session around, right?
          Hide
          Jukka Zitting added a comment -

          Do we want to include this already in 2.4? Seems like a relatively low-risk change so we could still backport this before the 2.4.0 release gets cut.

          Show
          Jukka Zitting added a comment - Do we want to include this already in 2.4? Seems like a relatively low-risk change so we could still backport this before the 2.4.0 release gets cut.
          Hide
          Julian Reschke added a comment -

          it's both low-risk and low-prio, so it might be something that can wait...

          Show
          Julian Reschke added a comment - it's both low-risk and low-prio, so it might be something that can wait...
          Hide
          Julian Reschke added a comment -

          (found to be needed in 2.4 as well so we can fix JCR-3633

          Show
          Julian Reschke added a comment - (found to be needed in 2.4 as well so we can fix JCR-3633

            People

            • Assignee:
              Julian Reschke
              Reporter:
              Julian Reschke
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development