Spring Security
  1. Spring Security
  2. SEC-1180

Unreachable code inside UrlUtils.buildRequestUrl(...)

    Details

    • Type: Refactoring Refactoring
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Won't Fix
    • Affects Version/s: 2.0.4, 3.0.0 M1
    • Fix Version/s: 3.0.0 RC1
    • Component/s: Web
    • Labels:
      None

      Description

      Noticed accidentally that the following fragment never gets executed:

      ----------------------------------
      if (uri == null)

      { uri = requestURI; uri = uri.substring(contextPath.length()); }

      ----------------------------------

      Before this block, the 'uri' is assigned the value of the servletPath which is never null (though it can be an empty string), according to the Servlets spec.

      Also from the spec: "It is important to note that, except for URL encoding differences between the
      request URI and the path parts, the following equation is always true:
      requestURI = contextPath + servletPath + pathInfo".

      So the method returns a correct result, just the the code may look a bit confusing. It may make sense just to remove the unreachable piece.

        Activity

        Hide
        Luke Taylor added a comment -

        I suspect this may just be there to prevent errors when using Spring's mock request objects but I'll take a look. Alternatively there may have been some historical reason (such as a container bug) which led to it being there.

        Show
        Luke Taylor added a comment - I suspect this may just be there to prevent errors when using Spring's mock request objects but I'll take a look. Alternatively there may have been some historical reason (such as a container bug) which led to it being there.
        Hide
        Luke Taylor added a comment -

        The tests all seem to pass Ok without it, so I've removed the block in question.

        Show
        Luke Taylor added a comment - The tests all seem to pass Ok without it, so I've removed the block in question.
        Hide
        Luke Taylor added a comment -

        It turns out this is actually used. In WebInvocationPrivilegeEvaluator, the requestURI is set directly on a mock request. In that case, the value of the servlet path is null.

        Show
        Luke Taylor added a comment - It turns out this is actually used. In WebInvocationPrivilegeEvaluator, the requestURI is set directly on a mock request. In that case, the value of the servlet path is null.
        Hide
        Luke Taylor added a comment -

        Reverted to code which checks the requestURI value as well as the servletPath if the latter is null. Modified to use a StringBuilder to create the result.

        Show
        Luke Taylor added a comment - Reverted to code which checks the requestURI value as well as the servletPath if the latter is null. Modified to use a StringBuilder to create the result.

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Anna Labinskaya
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: