Re: Re: IMS contribution
"Miguel Freitas" <[email protected]> Wed, 20 Dec 2006 11:51:17 +0000
| Newsgroups | gmane.comp.voip.nist-sip |
|---|---|
| Message-ID | <[email protected]> |
On 12/19/06, M. Rangnathan <[email protected]> wrote: > > Jeroen, Correct! Miguel, you may want to move the implementation classes > from gov.nist.javax.sip.header.ims to gov.nist.javax.sip.header to > avoid this problem. OK, I will do that! I'm almost done, but I'm still having some parser issues... One is the fact that parsing headers related with ParametersHeader (and using HeaderParser), some parameters appears off-order after encoding it: [junit] start test.unit.gov.nist.javax.sip.parser.ims.PChargingFunctionAddressesParserTest [junit] P-Charging-Function-Addresses: ccf=token; ecf="test quoted str" [junit] P-Charging-Function-Addresses: ecf="test quoted str"; ccf=token [junit] done test.unit.gov.nist.javax.sip.parser.ims.PChargingFunctionAddressesParserTest [junit] start test.unit.gov.nist.javax.sip.parser.ims.SecurityClientParserTest [junit] Security-Client: ipsec-3gpp; alg=hmac-sha-1-96; spi-c=23456789; spi-s=12345678; port-c=2468; port-s=1357 [junit] Security-Client: ipsec-3gpp; port-c=2468; spi-s=12345678; spi-c=23456789; port-s=1357; alg=hmac-sha-1-96 [junit] done test.unit.gov.nist.javax.sip.parser.ims.SecurityClientParserTest The other issue is with the PChargingFunctionAddresses header. The RFC3455 specifies that this header may contain several "ccf" and "ecf" parameters. eg: P-Charging-Function-Addresses: ccf=192.1.1.1; ccf=192.1.1.2; ecf= 192.1.1.3; ecf=192.1.1.4 So it seams that I can't use the NameValueList because in the end I only get one different named parameter value (the reference to get the parameter value is made by it's name): [junit] start test.unit.gov.nist.javax.sip.parser.ims.PChargingFunctionAddressesParserTest ! [junit] P-Charging-Function-Addresses: ccf=192.1.1.1; ccf=192.1.1.2; ecf=192.1.1.3; ecf=192.1.1.4 ! [junit] P-Charging-Function-Addresses: ecf=192.1.1.4; ccf=192.1.1.2 [junit] done test.unit.gov.nist.javax.sip.parser.ims.PChargingFunctionAddressesParserTest I think that one way to resolve this is to define a PChargingFunctionAddressesList list and add logic to it's parser so it adds a new PChargingFunctionAddresses header to the list if the next parameter already exists in the current PChargingFunctionAddresses header being parsed... Miguel Freitas On 12/19/06, M. Rangnathan <[email protected]> wrote: > > Jeroen, Correct! Miguel, you may want to move the implementation classes > from gov.nist.javax.sip.header.ims to gov.nist.javax.sip.header to > avoid this problem. You can leave the interfaces where they are. If you > use a refactoring tool (eclipse works for me), the move will be > painless. There are some other extension headers in > gov.nist.javax.sip.header.extension for which I shall do the same. > > Ranga > > > Jeroen van Bemmel wrote: > > > Miguel, > > > > It's a reflection error. SIPObject equals uses reflection, this works > > fine for headers that are in the gov.nist.javax.sip.header package but > > not in the subpackage ims. It works for Path because that header does > > not have any protected members in the ims subpackage > > > > Regards, > > Jeroen > > > > ----- Original Message ----- > > *From:* Miguel Freitas <mailto:[email protected]> > > *To:* M. Rangnathan <mailto:[email protected]> > > *Cc:* Jeroen van Bemmel <mailto:[email protected]> ; > > [email protected] <mailto:[email protected]> > > *Sent:* Monday, December 18, 2006 12:10 PM > > *Subject:* Re: [nist-sip] Re: IMS contribution > > > > > > Ranga, > > What could be the cause to this? > > > > [junit] Unexpected internal error FIXME!! Class > > gov.nist.javax.sip.header.SIPObject can not access a member of > > class gov.nist.javax.sip.header.ims.Privacy with modifiers > "protected" > > > > When testing the parsers I keep getting this error, even in the > > last cvs version and without chainging anything in the code except > > including the parser tests. It affects almost all headers > > contained in the gov.nist.javax.sip.header.ims package, except the > > Path header. > > > > [junit] start > > test.unit.gov.nist.javax.sip.parser.ims.PrivacyParserTest > > [junit] Privacy: none > > [junit] Privacy: none > > [junit] accessed field privacy > > [junit] modifier 4 > > [junit] modifier.private 2 > > [junit] Unexpected internal error FIXME!! Class > > gov.nist.javax.sip.header.SIPObject can not access a member of > > class gov.nist.javax.sip.header.ims.Privacy with modifiers > "protected" > > [junit] java.lang.IllegalAccessException: Class > > gov.nist.javax.sip.header.SIPObject can not access a member of > > class gov.nist.javax.sip.header.ims.Privacy with modifiers > "protected" > > [junit] at > > sun.reflect.Reflection.ensureMemberAccess(Reflection.java:65) > > [junit] at java.lang.reflect.Field.doSecurityCheck(Field.java > :954) > > [junit] at java.lang.reflect.Field.getFieldAccessor > > (Field.java:895) > > [junit] at java.lang.reflect.Field.get(Field.java:357) > > [junit] at > > gov.nist.javax.sip.header.SIPObject.equals(SIPObject.java:126) > > [junit] at java.util.AbstractList.equals(AbstractList.java :610) > > [junit] at > > gov.nist.javax.sip.header.SIPHeaderList.equals(SIPHeaderList.java > :429) > > [junit] at junit.framework.Assert.assertEquals(Assert.java:62) > > [junit] at junit.framework.Assert.assertEquals(Assert.java :71) > > [junit] at > > test.unit.gov.nist.javax.sip.parser.ParserTestCase.testParser( > ParserTestCase.java:74) > > [junit] at > > test.unit.gov.nist.javax.sip.parser.ims.PrivacyParserTest.testParser > (PrivacyParserTest.java:40) > > > > > > > > > > > > On 12/13/06, *M. Rangnathan* <[email protected] > > <mailto:[email protected]>> wrote: > > > > Miguel, > > > > What is the nature of the serious problem? Its a bit > > surprising. There > > have been no major changes in the parser for quite some time. > > You should > > however track all places where your older headers were being > > referenced. > > you should also use a refactoring tool (such as eclipse > > refactoring) to > > change class names. Otherwise, you will have to change things > > manually > > everywhere the old class name is referenced. I suppose you > > were already > > doing this but I thought I'd just mention it. > > > > There have been no major changes since that date. However, if > > you are > > using cvs and making changes against cvs, then a cvs update > before > > commit will merge any changes. I'd take a closer look before > > rushing to > > rewrite things. Your older contributions were working AFIK. > > This also > > points out the need for the test cases you are contributing > > this time. I > > could add these to the regression test suite to make sure > > nothing breaks > > in future once we sort this out. > > > > Regards > > > > Ranga > > > > > > > > Miguel Freitas wrote: > > > > > > > > I'm running it against CVS version downloaded at 2006-12-04, > > hope > > > nothing critical changed since... > > > And I made the changes with the help of Meld diff viewer, > > keeping (all > > > newer) changes intact. > > > > > > But I'm having serious problems with the IMS headers > > parsers, that I'm > > > considering re-writing them from scratch... :( > > > That is strange, because I was using all of them to build my > > SIP-IMS > > > UA without any problem (older version of JSIP1.2)... > > Anything has > > > changed concerning the parsing that I might be missing? > > > > > > > > > Miguel Freitas > > > - - > > > - - - - - - - - > > > - Telecommunications Institute - > > > - Instituto de Telecomunicacoes - > > > - Aveiro, Portugal - > > > - www.it.pt <http://www.it.pt> > > <http://www.it.pt> - > > > - - - - - - - - > > > > > > On 12/12/06, *M. Ranganathan* < [email protected] > > <mailto:[email protected]> > > > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > > > Jeroen, > > > > > > OK. Thanks for informing. I fixed the problem ( well, at > > least I > > > did an > > > unset CLASSPATH and tried ant complieri and compiletck > > and it seems to > > > be OK now). > > > > > > Miguel, OK please go ahead and change names if you wish. > > Please > > > use the > > > current version and change its name so as to keep the > latest > > > changes (if > > > any) intact. > > > > > > Please go ahead and commit after running tck and I'll > > announce your > > > contribution on the list (which I have already done in > > effect :-) ). > > > Thanks for writing up an example. > > > > > > Regards, > > > > > > > > > Ranga > > > > > > > > > > > > On Tue, 2006-12-12 at 20:13 +0100, Jeroen van Bemmel > wrote: > > > > Specifically, I need to do this for 'ant runtck'. > > Build target works > > > > > > > > Jeroen > > > > ----- Original Message ----- > > > > From: Jeroen van Bemmel > > > > To: Miguel Freitas ; M. Ranganathan > > > > Cc: [email protected] > > <mailto:[email protected]> <mailto:[email protected] > > <mailto:[email protected]>> > > > > Sent: Tuesday, December 12, 2006 7:10 PM > > > > Subject: Re: [nist-sip] Re: IMS contribution > > > > > > > > > > > > I always have to do 'set > > CLASSPATH=lib/junit-x.x.x.jar' > > > to get > > > > it to build under ant (from a commandline > window) > > > > > > > > Jeroen > > > > ----- Original Message ----- > > > > From: Miguel Freitas > > > > To: M. Ranganathan > > > > Cc: [email protected] > > <mailto:[email protected]> > > > <mailto:[email protected] > > <mailto:[email protected]>> > > > > Sent: Tuesday, December 12, 2006 6:17 PM > > > > Subject: [nist-sip] Re: IMS contribution > > > > > > > > > > > > > > > > On 12/12/06, M. Ranganathan < > > [email protected] <mailto:[email protected]> > > > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > Hi Miguel, > > > > > > > > > > > > I really cant figure out the > > reasons for > > > your > > > > compile problems. I just > > > > checked out a fresh copy and I > > still have no > > > > problems. Does anybody else > > > > on this list have such problems? > > > Clearly, its > > > > because you dont have > > > > junit in your class path but > > why? It > > > does not > > > > seem to be an issue for > > > > me. > > > > > > > > Yes, it seams to be a class path issue > > to ant, > > > since > > > > junit.framework.* is accessible inside > > Eclipse. > > > > Is there other way to compile the > > parser tests? > > > > Silly question but.. Is there anything > > I have to > > > > install on my system or put in the > > system CLASSPATH? > > > > > > > > The "compileunit" target does not > > include, like > > > for eg > > > > "compiletck" does: > > > > <classpath> > > > > <pathelement > > > > location='${junit_jar}' /> > > > > </classpath> > > > > could this issue be related to it? > > > > > > > > > > > > -> P-Headers names changed to > > include a > > > > leading "P". eg: > > > > PAccessNetworkInfoHeader.java > > > > > > > > Any reason for the class name > > changes? It > > > > could cause issues with > > > > people that are already using > your > > > > contributions. > > > > > > > > I know it's not a developer-friendly > > decision, > > > and not > > > > a so good policy to change stuff names > > after > > > > deployment, but I think that sooner or > > later > > > someone > > > > will want to change those names since > > their real > > > name > > > > includes a leading "P". > > > > I share most of the responsibility on > that > > > issue, and > > > > I should had changed it as soon as I > > started > > > working > > > > on them and before their first > submission. > > > > But that's my humble opinion. What do > > you think? > > > > > > > > > > > > > > > > A small nit: can you also > > please fix the > > > > javadoc errors in your code > > > > ( there are some from the > > previous > > > > contributions you made). > > > > > > > > Yes, of course. Is being taking care of. > > > > > > > > > > > > Thanks for adding the unit > > tests. Your > > > > contribution would be a good > > > > basis to pattern IMS headers > > for JSIP 1.3 . > > > > > > > > Best regards, > > > > > > > > Ranga. > > > > > > > > > > > > > > > > On Tue, 2006-12-12 at 15:54 > > +0000, Miguel > > > > Freitas wrote: > > > > > > > > > > sorry, I forgot to change > > the subject :) > > > > > > > > > > On 12/12/06, M. Ranganathan > > > > <[email protected] > > <mailto:[email protected]> > > > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > > > > > > > > > > > > Hi Miguel, > > > > > > > > > > I look forward to your > > > > contributions! > > > > > > > > > > I managed to include the > > changes in a > > > recent > > > > CVS source code version > > > > > and I'm just concluding the > > parser > > > tests and > > > > a simple example for the > > > > > usage of the new headers. > > Hence my > > > need to > > > > compile the unit tests. > > > > > > > > > > Here's a list of my changes, > > please let me > > > > know if at this point you > > > > > might not agree with any of > > them.. > > > > > > > > > > > > ----------------------------------------- > > > > > .: changes in JAIN-SIP RI > > > > > > > ----------------------------------------- > > > > > > > > > > -> changes in > > > > gov.nist.core.NameListCore.java > > > > > . white space added > > between the > > > several > > > > parameters encoding > > > > > > > > > > -> added > > > > > > > gov.nist.javax.sdp.fields.PreconditionFields.java > > > > > -> changes in > > > > > > gov.nist.sdp.MediaDescriptionImpl.java > > > > > . changed > > BandwithField logic > > > > > . PUBLIC > > > addAttribute(AttributeField > > > > af) > > > > > - is there other > > way to add > > > > several SDP attributes with > > > > > the same name to the same > Media > > > Description? > > > > > eg: m=video > > 00 RTP/AVP 98 > > > > 99 > > > > > > > a=rtpmap:98 H263 > > > > > > > a=rtpmap:99 MP4V-ES > > > > > > > > > > -> P-Headers names changed > > to include a > > > > leading "P". eg: > > > > > PAccessNetworkInfoHeader.java > > > > > > > > > > -> changes in > > > > > > > gov.nist.javax.sip.header.ims.ParametersNamesIms.java > > > > > > > > > > -> changes in > > > > > > > gov.nist.javax.sip.header.ims.SIPHeaderNamesIms.java > > > > > > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityAgree.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityAgreeHeader.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityClient.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityClientList.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityClientHeader.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityServer.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityServerList.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityServerHeader.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityVerify.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityVerifyList.java > > > > > -> added > > > > > > > gov.nist.javax.sip.header.ims.SecurityVerifyHeader.java > > > > > > > > > > -> changes in > > > > > > > gov.nist.javax.sip.header.HeaderFactoryImpl.java > > > > > > > > > > -> changes in > > > > > > gov.nist.javax.sip.header.NameMap.java > > > > > > > > > > -> changes in > > > > > > gov.nist.javax.sip.message.ListMap.java > > > > > > > > > > -> changes in > > > > > > gov.nist.javax.sip.parser.ParserFactory.java > > > > > > > > > > -> changes in > > > > > > gov.nist.javax.sip.parser.Lexer.java > > > > > > > > > > -> changes in > > > > > > gov.nist.javax.sip.parser.TokenTypes.java > > > > > > > > > > -> added > > > > > > > gov.nist.javax.sip.parser.PAssociatedURIParser.java > > > > > -> added > > > > > > > gov.nist.javax.sip.parser.PCalledPartyIDParser.java > > > > > -> added > > > > > > > gov.nist.javax.sip.parser.SecurityAgreeParser.java > > > > > -> added > > > > > > > gov.nist.javax.sip.parser.SecurityClientParser.java > > > > > -> added > > > > > > > gov.nist.javax.sip.parser.SecurityServerParser.java > > > > > -> added > > > > > > > gov.nist.javax.sip.parser.SecurityVerifyParser.java > > > > > > > > > > -> added > > > > > > unit.gov.nist.javax.sip.parser.ims.* > > > > > -> added > > > > > > > unit.gov.nist.javax.sip.parser.ims.SecurityClientTest.java > > > > > -> added > > > > > > > unit.gov.nist.javax.sip.parser.ims.SecurityServerTest.java > > > > > ... > > > > > > > > > > -> added examples.ims.* > > > > > . based on > > examples.simplecallsetup > > > > > > > > > > > > > > > > > ----------------------------------------- > > > > > .: changes in proprietary > > JAIN-SIP API > > > > > > > ----------------------------------------- > > > > > > > > > > -> changes in > > > > javax.sdp.MediaDescription.java > > > > > . methods for > > PreconditionFields > > > > > . PUBLIC > > > addAttribute(AttributeField af) > > > > > > > > > > > > > > > > > > > > > > > > > Miguel Freitas > > > > > -- > > > > > - - - - > > > > - - - - > > > > > - Telecommunications > > Institute - > > > > > - Instituto de > > Telecomunicacoes - > > > > > - Aveiro, > > > Portugal - > > > > > - > > > > www.it.pt <http://www.it.pt> > > > <http://www.it.pt> - > > > > > - - - - > > > > - - - - > > > > > > > > > > > > > > > -- > > > > > - - - - > > > > - - - - > > > > > - Telecommunications > > Institute - > > > > > - Instituto de > > Telecomunicacoes - > > > > > - Aveiro, > > > Portugal - > > > > > - > > > > www.it.pt <http://www.it.pt> > > > < http://www.it.pt> - > > > > > - - - - > > > > - - - - > > > > > > > > > > > > > > > > > > > > -- > > > > - - - > > - - - - - > > > > - Telecommunications Institute - > > > > - Instituto de Telecomunicacoes - > > > > - Aveiro, > > Portugal - > > > > - www.it.pt <http://www.it.pt> > > > <http://www.it.pt> - > > > > - - - > > - - - - - > > > > > > > > > > > > > > > ______________________________________________________ > > > > > > > > > > _______________________________________________ > > > > nist-sip mailing list > > > > [email protected] > > <mailto:[email protected]> > > > <mailto:[email protected] > > <mailto:[email protected]>> > > > > > > http://www-x.antd.nist.gov/mailman/listinfo/nist-sip > > <http://www-x.antd.nist.gov/mailman/listinfo/nist-sip> > > > > > > > > > > > > > > > > > ______________________________________________________________ > > > > > > > > _______________________________________________ > > > > nist-sip mailing list > > > > [email protected] > > <mailto:[email protected]> <mailto:[email protected] > > <mailto:[email protected]>> > > > > > > http://www-x.antd.nist.gov/mailman/listinfo/nist-sip > > > > > > > > > > > > -- > > M. Ranganathan > > > > Advanced Networking Technologies Division, > > National Institute of Standards and Technology (NIST), > > 100 Bureau Drive, Stop 8920, Gaithersburg, MD 20899. > > tel:301 975 3664 , fax:301 590 0932 http://w3.antd.nist.gov/ > > Advanced Networking Technologies For the People! > > > > > > > > > -- > M. Ranganathan > > Advanced Networking Technologies Division, > National Institute of Standards and Technology (NIST), > 100 Bureau Drive, Stop 8920, Gaithersburg, MD 20899. > tel:301 975 3664 , fax:301 590 0932 http://w3.antd.nist.gov/ > Advanced Networking Technologies For the People! > > > _______________________________________________ nist-sip mailing list [email protected] http://www-x.antd.nist.gov/mailman/listinfo/nist-sip