Re: Re: IMS contribution
"Jeroen van Bemmel" <[email protected]> Wed, 20 Dec 2006 14:17:21 +0100
| Newsgroups | gmane.comp.voip.nist-sip |
|---|---|
| Message-ID | <004c01c72439$2ecb26f0$0601a8c0@BEMBUSTER> |
Miguel, Per RFC3261 the order of parameters is insignificant, and a parameter name may appear only once. Some IMS headers violate these assumptions, so indeed you cannot use ParametersHeader / NameValueList for those Regards, Jeroen ----- Original Message ----- From: Miguel Freitas To: M. Rangnathan Cc: Jeroen van Bemmel ; [email protected] Sent: Wednesday, December 20, 2006 12:51 PM Subject: Re: [nist-sip] Re: IMS contribution 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