SEC-1180: Unreachable code inside UrlUtils.buildRequestUrl(...) #1428

Closed
spring-issuemaster opened this Issue Jun 12, 2009 · 4 comments

1 participant

@spring-issuemaster

Anna Labinskaya (Migrated from SEC-1180) said:

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.

@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster

Luke Taylor said:

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

@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster spring-issuemaster added this to the 3.0.0 RC1 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment