Impossible branch in o.a.t.websocket.server.WsServerContainer.addEndpoint

[Christopher Schultz]

All,

I was looking at https://bz.apache.org/bugzilla/show_bug.cgi?id=57676 (a
simple proposed patch to improve an error message) and I was trying to
figure out what to do with a bit of code in addEndpoint. Reading the
code, I think I've spotted a problem:

[187]   UriTemplate uriTemplate = new UriTemplate(path);
        if (uriTemplate.hasParameters()) {
            Integer key = Integer.valueOf(uriTemplate.getSegmentCount());
            SortedSet<TemplatePathMatch> templateMatches =
                    configTemplateMatchMap.get(key);
            if (templateMatches == null) {
                // Ensure that if concurrent threads execute this block they
                // both end up using the same TreeSet instance
                templateMatches = new TreeSet<>(
                        TemplatePathMatchComparator.getInstance());
                configTemplateMatchMap.putIfAbsent(key, templateMatches);
                templateMatches = configTemplateMatchMap.get(key);
            }
[200]       if (!templateMatches.add(new TemplatePathMatch(sec,
uriTemplate))) {
                // Duplicate uriTemplate;
                throw new DeploymentException(
                        sm.getString("serverContainer.duplicatePaths", path,
                                     sec.getEndpointClass(),
                                     sec.getEndpointClass()));
            }
        }

The problem is on line 200, where we check to see if
templateMatches.add() returns false (meaning that the object we added to
the set replaced one that was already there).

I don't believe this branch can /ever/ be reached because the
TemplatePathMatch class doesn't override Object.equals(), thus no two
objects of that type will ever equal each other. Therefore, a new
TemplatePathMatch object will always be added to the set.

I'm not sure what the implications are of multiple TemplatePathMatch
objects being in that set, but it looks like it's bad (we throw an
exception in that case), so someone with a better understanding of such
things might want to take a look.

(Also, if anyone would care to comment on how to get the "pre-existing"
match for the error message -- which is different than current-trunk as
it's what I'm working on -- I'd love some insight.)

Thanks,
-chris

signature.asc
Description: OpenPGP digital signature