To Review
22 Reviews
Reviews
Expand All |
Collapse All
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-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-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-16
Session size optimization using Externalizable
Session size optimization using Externalizable
Review
DEMO-9
Join/Replace Changes.
Summary of changes. ----------------- 1. RFC 3891 and 3911 are implemented. Session Targetting based on Join/Replace is implemented as per JSR 289. The changes are applicable only to UA and not to Proxy. 2. 3911: An incoming request to the server will contain Join header with the callid/fromtag/totag of the call it want to join. Sailfin will make sure that the session of the joining request is added to the SAS of the session specified by the Join Header. 3. 3891: Same as 2 except that the header is Repaces and not Join. Main difference is the ability to specify the early-flag. 4. Both RFCs talk about authentication requirements. My thinking is that with the current digest auth support, the requirements can be met by the application. 5. SipSessionsUtil.getCorrespondingSession is implemented. 6. SAS.getApplicationName is implemented (it was a simple change). 7. For 3911, the RFC explains how the original request will be terminated. My thinking is that the termination will be done by the application and not by the container. Thats the reason why 289 have the new SipSessionsUtil.getCorrespondingSession api. 8. The request coming in to the container with Join/Replace will not have fragment-id. So, to searching dialog is a problem. Would join/rep lace be applicable in case of spiralling? probably not. Let me know, if there is a way to introduce fid in Join/Replace requests. 10. I have added 5 devtests. FT seems to be fine.