On 17/03/2015 14:45, Christopher Schultz wrote: > Ping Sorry. This dropped off my radar while I was trying to figure out the APR crash.
I'll take a look later today. Mark > > 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 >> > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
