Crucible

Review SFIN-40 Review of changes for 1418, 1368 and 1446 in the FCS branch Review changes for issues 1418, 1368 and 1446 to be introduced in the FCS branch.

Review SFIN-39 Review changes for issue 1368 and 1446 In the course of implementing the changes for issue 1368 and 1446, I also found some other irregularities in the code, which I have fixed: DcrUtils.java ------------ Just changed name of a field Connection.java --------------- Introduced a dummy connection that is used mark that UDP was used as transport (see more about that below). SipLoadBalancerBackend.java ---------------------------- General: - Changed log levels and removed logging of message contents (also removed some other superfluous loggings). Line 100-101: - Removed argument hashKeyExtractor since it will not be needed anymore, according to issue 1368. - moved call to LoadBalancerUtil.generateInstanceID() outside the class to make it unit testable. Line 152-182: - Removed setting of bekey attribute on application session and message according to issue 1368. Line 226-231: - Made it more strict: in the case of proxying there shall always exist a connection. Line 261-267: - Made it more strict: in the case of proxying there shall always exist a connection. Line 414-434: - Simplified getHashKey() according to issue 1368; will only read hash key (bekey) from application session. Line 457-460: - Removed getHashKeyExtractor() since it is not needed anymore, according to issue 1368. Line 520-524: - Changed to use the correct methods for setting the client certificate on a message. Line 530: - Fix for 1446: set CONNID_ATTR as a system attribute on the message. SipLoadBalancerIncomingHandler.java ------------------------------------ General: - Changed log levels and removed logging of message contents (also removed some other superfluous loggings). Line 83-86: - Moved call to LoadBalancerUtil.getLocalSocket() outside the class to make it unit testable. Line 217-225: - Fixed handling of missing connection (connid parameter on Via); the check is now stricter and does not allow the 'connid' parameter to be missing. The 'connid' parameter will be set also when the client was using UDP, but in this case the new Connection.UDP connection will be encoded and in this case the connection will be set using the resolved Via. The absence of the 'connid' parameter indicates some program error in the container and now renders an exception. Line 348-353: - Fix related to line 217-225: set the 'connid' also if UDP was used. Line 384-387: - Removed unused method SipLoadBalancerManagerBackEnd.java ------------------------------------ Line 169-195: - Fixes for 1368, since DCR shall never be applied on outgoing messages (will always be set on the application session at creation of it), the creation and setting of the hash key extractor is removed. Also change for making the SipLoadBalancerBackend unit testable. SipRequestGroup.java --------------------- Change to make SipLoadBalancerIncomingHandler unit testable. RecordRouteResolver.java ------------------------- Line 157 and 190: - The CLB will indirectly execute these lines will execute for responses that has no request attached to it yet (the request is attached in the transaction manager layer, but the CLB layer is placed before before it). SipServletMessageImpl.java -------------------------- Line 95-96: - Made the class possible to use in unit tests: removed singleton calls to dialog manager in various methods and instead placed the dialog manager in a field that can be set (to a mock object) using reflection in the unit test driver. Line 1169-1282: - Fix for 1446: added method for setting system attributes (which are copied at cloning) SipServletRequestImpl.java -------------------------- Changed usage of dialog manager singleton. SipServletResponseImpl.java -------------------------- Changed usage of dialog manager singleton. TargetResolver.java ------------------- Changed setting of DnsResolver singleton in the static initialization to the getInstance() method to make it possible to use the target resolver in unit tests with a mocked DnsResolver.

Review SFIN-37 Issue 1147

Review SFIN-36 Issue 1350

Review SFIN-35 Review changes for issue 1365 review changes for issue 1365: https://sailfin.dev.java.net/issues/show_bug.cgi?id=1365

Review SFIN-34 Review changes for issue 1367 Review changes for issue 1367: https://sailfin.dev.java.net/issues/show_bug.cgi?id=1367

Review SFIN-33 Code review: Changes for issue 1220 Review changes for issue 1220: https://sailfin.dev.java.net/issues/show_bug.cgi?id=1220

Review SFIN-32 Code review: correction of issue 1362 Review the changes done to fix issue 1362: https://sailfin.dev.java.net/issues/show_bug.cgi?id=1362

Review SFIN-31 Issue 846 471

Review SFIN-29 Issue 1325

Review SFIN-28 Issue 1322

Review SFIN-27 Issue 1303

Review SFIN-26 Application Names as per section 7.6, JSR 289. How the application name need to be used is defined in section 7.6 of JSR 289. Application Routers would expect the application names as per the explanation there. Also, lookup of the SipFactory, SipSessionsUtil also will be based on the appname as mentioned in section 7.6. i.e the jndi name of SipFactory will be sip/<appname>/SipFactory SailFin has been using context.getName as the application name for quite some time now. However, context.getName typically is the context root and is used like that in the tomcat code. So, context.getName will be used in the tomcat container for many things. The fix is to set context.getAppName as per section 7.6 of the JSR 289 and use it everywhere in the container instead of context.getName. Notes: 1) I did not understand completely getApplicationId() logic, especially the parent traversal and appending the container names in the application id. 2) I did not understand completely how AuthModule uses the appname. Hope Jan and Venu can review that part of the code.

Review SFIN-25 Code changes for issue 1265 Review code changes for issue 1265 (https://sailfin.dev.java.net/issues/show_bug.cgi?id=1265)

Review SFIN-23 NPE with sipbomber (issue 1281 1282)

Review SFIN-21 Issue 980

Review SFIN-19 Fixes for TCK issues Following are the issues fixed. 1. SipServletMessageImpl.setContent(content, type) does not honor charset in the text/plain content-type. This is because the decoded character encoding is set, after the bytes are processed. Affected file is SipServletMessageImpl.java. 2. B2buaHelperImpl.java should throw an invalidargumentexception, if the session passed in to unlinkSessions is invalid. 3. when Proxy.setAddtoPath for REGISTER requests, the container throw an exception if the request does not contain "path" in the "Supported" header. However as per setAddtoPath javadoc, it is a best practice for the servlet to check whether "path" is present in the Supported header. 4. ProxyBranch.isStarted now uses its own "isStarted" flag. 5. committed state checking now, includes a check for presence of transaction request. If transaction request is present, (i.e, the clonned request has a transaction request), then check its committed status first. 6. markReadyToInvalidate logic in updateSipSessionState method of SipSessionImplBase now take care of proxy branches. - When a 601>status>299 response for a proxy branch reach on server, now the server checks if that is the last response the proxy is expecting (i.e still dont have 200 response from any of the branches and this is the last branch). - If atleast one proxy branch is has record-route true, the 200 response wont mark the session for invalidation. 7. reset() method is now applicable for all repsonses, when state is INITIAL. 8. If servlet do getProxyBranch(uri), right after crateProxyBranches(list), the code was failing. That was fixed. 9. I have temperorily reverted Jie's latest changes on multihoming to get the tests passing. It wont be checked in. It is just for me to get a clean test run. ~ ~

Review SFIN-15 Fix for Application Name and Main Servlet Issues

Review SFIN-14 TelURL Changes I fixed TelURL parser and made it more stricter.

Review SFIN-11 Quicklook version

Summarize SFIN-10 Various annotations