Author: markt Date: Thu Jan 31 14:27:17 2019 New Revision: 1852597 URL: http://svn.apache.org/viewvc?rev=1852597&view=rev Log: Implement the requirements of section 5.2.1 of the WebSocket 1.1 specification and ensure that if the deployment of one Endpoint fails, no Endpoints are deployed for that web application.
Modified: tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties tomcat/trunk/java/org/apache/tomcat/websocket/server/WsServerContainer.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties?rev=1852597&r1=1852596&r2=1852597&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties [UTF-8] (original) +++ tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties [UTF-8] Thu Jan 31 14:27:17 2019 @@ -17,6 +17,7 @@ serverContainer.addNotAllowed=No further serverContainer.configuratorFail=Failed to create configurator of type [{0}] for POJO of type [{1}] serverContainer.duplicatePaths=Multiple Endpoints may not be deployed to the same path [{0}] : existing endpoint was [{1}] and new endpoint is [{2}] serverContainer.encoderFail=Unable to create encoder of type [{0}] +serverContainer.failedDeployment=Deployment of WebSocket Endpoints to the web application with path [{0}] in host [{1}] is not permitted due to the failure of a previous deployment serverContainer.missingAnnotation=Cannot deploy POJO class [{0}] as it is not annotated with @ServerEndpoint serverContainer.missingEndpoint=An Endpoint instance has been request for path [{0}] but no matching Endpoint class was found serverContainer.pojoDeploy=POJO class [{0}] deploying to path [{1}] in ServletContext [{2}] Modified: tomcat/trunk/java/org/apache/tomcat/websocket/server/WsServerContainer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/server/WsServerContainer.java?rev=1852597&r1=1852596&r2=1852597&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/websocket/server/WsServerContainer.java (original) +++ tomcat/trunk/java/org/apache/tomcat/websocket/server/WsServerContainer.java Thu Jan 31 14:27:17 2019 @@ -80,6 +80,7 @@ public class WsServerContainer extends W private volatile boolean addAllowed = true; private final Map<String,Set<WsSession>> authenticatedSessions = new ConcurrentHashMap<>(); private volatile boolean endpointsRegistered = false; + private volatile boolean deploymentFailed = false; WsServerContainer(ServletContext servletContext) { @@ -126,8 +127,7 @@ public class WsServerContainer extends W * requested */ @Override - public void addEndpoint(ServerEndpointConfig sec) - throws DeploymentException { + public void addEndpoint(ServerEndpointConfig sec) throws DeploymentException { if (enforceNoAddAfterHandshake && !addAllowed) { throw new DeploymentException( @@ -138,50 +138,61 @@ public class WsServerContainer extends W throw new DeploymentException( sm.getString("serverContainer.servletContextMissing")); } - String path = sec.getPath(); - // Add method mapping to user properties - PojoMethodMapping methodMapping = new PojoMethodMapping(sec.getEndpointClass(), - sec.getDecoders(), path); - if (methodMapping.getOnClose() != null || methodMapping.getOnOpen() != null - || methodMapping.getOnError() != null || methodMapping.hasMessageHandlers()) { - sec.getUserProperties().put(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY, - methodMapping); - } - - 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); - } - if (!templateMatches.add(new TemplatePathMatch(sec, uriTemplate))) { - // Duplicate uriTemplate; - throw new DeploymentException( - sm.getString("serverContainer.duplicatePaths", path, - sec.getEndpointClass(), - sec.getEndpointClass())); + if (deploymentFailed) { + throw new DeploymentException(sm.getString("serverContainer.failedDeployment", + servletContext.getContextPath(), servletContext.getVirtualServerName())); + } + + try { + String path = sec.getPath(); + + // Add method mapping to user properties + PojoMethodMapping methodMapping = new PojoMethodMapping(sec.getEndpointClass(), + sec.getDecoders(), path); + if (methodMapping.getOnClose() != null || methodMapping.getOnOpen() != null + || methodMapping.getOnError() != null || methodMapping.hasMessageHandlers()) { + sec.getUserProperties().put(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY, + methodMapping); } - } else { - // Exact match - ServerEndpointConfig old = configExactMatchMap.put(path, sec); - if (old != null) { - // Duplicate path mappings - throw new DeploymentException( - sm.getString("serverContainer.duplicatePaths", path, - old.getEndpointClass(), - sec.getEndpointClass())); + + 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); + } + if (!templateMatches.add(new TemplatePathMatch(sec, uriTemplate))) { + // Duplicate uriTemplate; + throw new DeploymentException( + sm.getString("serverContainer.duplicatePaths", path, + sec.getEndpointClass(), + sec.getEndpointClass())); + } + } else { + // Exact match + ServerEndpointConfig old = configExactMatchMap.put(path, sec); + if (old != null) { + // Duplicate path mappings + throw new DeploymentException( + sm.getString("serverContainer.duplicatePaths", path, + old.getEndpointClass(), + sec.getEndpointClass())); + } } - } - endpointsRegistered = true; + endpointsRegistered = true; + } catch (DeploymentException de) { + failDeployment(); + throw de; + } } @@ -195,43 +206,59 @@ public class WsServerContainer extends W @Override public void addEndpoint(Class<?> pojo) throws DeploymentException { - ServerEndpoint annotation = pojo.getAnnotation(ServerEndpoint.class); - if (annotation == null) { - throw new DeploymentException( - sm.getString("serverContainer.missingAnnotation", - pojo.getName())); - } - String path = annotation.value(); + ServerEndpointConfig sec; - // Validate encoders - validateEncoders(annotation.encoders()); + try { + ServerEndpoint annotation = pojo.getAnnotation(ServerEndpoint.class); + if (annotation == null) { + throw new DeploymentException( + sm.getString("serverContainer.missingAnnotation", + pojo.getName())); + } + String path = annotation.value(); - // ServerEndpointConfig - ServerEndpointConfig sec; - Class<? extends Configurator> configuratorClazz = - annotation.configurator(); - Configurator configurator = null; - if (!configuratorClazz.equals(Configurator.class)) { - try { - configurator = annotation.configurator().getConstructor().newInstance(); - } catch (ReflectiveOperationException e) { - throw new DeploymentException(sm.getString( - "serverContainer.configuratorFail", - annotation.configurator().getName(), - pojo.getClass().getName()), e); + // Validate encoders + validateEncoders(annotation.encoders()); + + // ServerEndpointConfig + Class<? extends Configurator> configuratorClazz = + annotation.configurator(); + Configurator configurator = null; + if (!configuratorClazz.equals(Configurator.class)) { + try { + configurator = annotation.configurator().getConstructor().newInstance(); + } catch (ReflectiveOperationException e) { + throw new DeploymentException(sm.getString( + "serverContainer.configuratorFail", + annotation.configurator().getName(), + pojo.getClass().getName()), e); + } } + sec = ServerEndpointConfig.Builder.create(pojo, path). + decoders(Arrays.asList(annotation.decoders())). + encoders(Arrays.asList(annotation.encoders())). + subprotocols(Arrays.asList(annotation.subprotocols())). + configurator(configurator). + build(); + } catch (DeploymentException de) { + failDeployment(); + throw de; } - sec = ServerEndpointConfig.Builder.create(pojo, path). - decoders(Arrays.asList(annotation.decoders())). - encoders(Arrays.asList(annotation.encoders())). - subprotocols(Arrays.asList(annotation.subprotocols())). - configurator(configurator). - build(); addEndpoint(sec); } + void failDeployment() { + deploymentFailed = true; + + // Clear all existing deployments + endpointsRegistered = false; + configExactMatchMap.clear(); + configTemplateMatchMap.clear(); + } + + boolean areEndpointsRegistered() { return endpointsRegistered; } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1852597&r1=1852596&r2=1852597&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Thu Jan 31 14:27:17 2019 @@ -201,6 +201,11 @@ <fix> Ignore synthetic methods when scanning POJO methods. (markt) </fix> + <fix> + Implement the requirements of section 5.2.1 of the WebSocket 1.1 + specification and ensure that if the deployment of one Endpoint fails, + no Endpoints are deployed for that web application. (markt) + </fix> </changelog> </subsection> <subsection name="Web applications"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org