Ping On 3/9/15 12:19 PM, Christopher Schultz wrote: > 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
