incubator-yoko-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Darren Middleman" <dmiddle...@gmail.com>
Subject Re: Unsigned short IdlToWsdl corba binding type tests
Date Wed, 14 Feb 2007 11:48:54 GMT
Hello Matteo,

I've had a look at the proposed changes and they look good.  The only
question I had was
whether there needs to be a similar change on the writeUShort side?  I'll
need to look at that
part of code to make sure that everything is OK on that side, but don't
worry about that for now.

I think I'm also going to make a similar sort of change to the read/write
character method that I
made earlier this week.  I'm going to make it follow the approach you have
listed here since it
handles the bounds wrapping case better than the one I had initially
committed.

Cheers,
Darren


On 2/13/07, Vescovi, Matteo <matteo.vescovi@iona.com> wrote:
>
> Hi,
> it turned out that the problem was in
>
> bindings/src/main/java/org/apache/yoko/bindings/corba/CorbaObjectReader.java
>
> Attached is a patch that fixes the problem - apply by copying patch file
> in trunk directory and running `patch -p0 <
> idlwtowsdl_unsigned_short_type_test.diff' from trunk.
>
> I'd like someone with a better understanding of the corba binding and
> runtime module to give me the ok before I commit the change.
>
> Cheers,
> - Matteo
>
>
>
>
> Vescovi, Matteo wrote:
> > Hi,
> > after looking a bit more into this, I think the problem could actually
> > be in core/src/main/java/org/apache/yoko/orb/CORBA/InputStream.java
> >
> > I think the marshalling of the unsigned short is correct, but when we
> > unmarshall the value in the InputStream reader we reuse the read_short
> > method, which assumes we are reading a signed short.
> >
> > Cheers,
> > - Matteo
> >
> >
> >
> > Vescovi, Matteo wrote:
> >> Hi,
> >> I'm working on the unsigned short idltowsdl corba binding type test.
> >>
> >> When sending an unsigned short greater than 2^15, the value is
> >> incorrectly marshalled to a negative value. I tracked down the
> >> problem to an implicit conversion between Integer and short types
> >> occuring between the CorbaObjectWriter.writeUShort(Integer)
> >> invocation and OutputStream.write_ushort(short) (see stack trace
> below).
> >>
> >> Thread [main] (Suspended)      OutputStream.write_short(short) line:
> >> 894      OutputStream.write_ushort(short) line: 900
> >> CorbaObjectWriter.writeUShort(Integer) line: 155
> >> CorbaObjectWriter.write(CorbaObjectHandler) line: 73
> >> CorbaStreamable._write(OutputStream) line: 57
> >> Any.write_value(OutputStream) line: 482      Request.marshal() line:
> >> 96      Request.invoke() line: 291
> >> CorbaConduit.buildRequest(CorbaMessage, OperationType) line: 187
> >> CorbaConduit.close(Message) line: 128
> >> MessageSenderInterceptor.handleMessage(Message) line: 58
> >> PhaseInterceptorChain.doIntercept(Message) line: 167
> >> ClientImpl.invoke(BindingOperationInfo, Object[], Map<String,Object>)
> >> line: 152      JaxWsClientProxy(ClientProxy).invokeSync(Method,
> >> BindingOperationInfo, Object[], Map<String,Object>) line: 73
> >> JaxWsClientProxy.invoke(Object, Method, Object[]) line: 116
> >> $Proxy37.testUnsignedShort(int, Holder, Holder) line: not available
> >>
> >> IdlToWsdlTypeTest(AbstractIdlToWsdlTypeTestClient).testUnsignedShort()
> >> line: 184      NativeMethodAccessorImpl.invoke0(Method, Object,
> >> Object[]) line: not available [native method]
> >> NativeMethodAccessorImpl.invoke(Object, Object[]) line: 39
> >> DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 25
> >> Method.invoke(Object, Object...) line: 585
> >> IdlToWsdlTypeTest(TestCase).runTest() line: 164
> >> IdlToWsdlTypeTest(TestCase).runBare() line: 130
> >> TestResult$1.protect() line: 106      TestResult.runProtected(Test,
> >> Protectable) line: 124      TestResult.run(TestCase) line: 109
> >> IdlToWsdlTypeTest(TestCase).run(TestResult) line: 120
> >> TestSuite.runTest(Test, TestResult) line: 230
> >> TestSuite.run(TestResult) line: 225      TestRunner.doRun(Test,
> >> boolean) line: 121      TestRunner.doRun(Test) line: 114
> >> TestRunner.run(Test) line: 77      TestRunner.run(Class) line:
> >> 62      IdlToWsdlTypeTest.main(String[]) line: 59  According to the
> >> CORBA 2.6 spec, unsigned short's range is 0...2^16 - 1
> >>
> >> I am going to fix this, and then post a patch for review before I
> >> commit the changes.
> >>
> >> Cheers,
> >> - Matteo
> >>
> >>
> >> Darren Middleman wrote:
> >>> Hello Matteo,
> >>>
> >>> For most of the basic types, this information should be in the third
> >>> chapter
> >>> of the CORBA spec, under IDL Syntax and Semantics.  The information is
> >>> a bit spread out over the chapter but it does give a good idea of
> >>> the valid
> >>> ranges for each of the types.
> >>>
> >>> Cheers,
> >>> Darren
> >>>
> >>>
> >>>
> >>> On 2/12/07, Vescovi, Matteo <matteo.vescovi@iona.com> wrote:
> >>>>
> >>>> That's great, thanks Darren.
> >>>>
> >>>> I debugged into that part of code and saw the char being assigned
> >>>> an out
> >>>> of range value, but I wasn't sure what the appropriate encoding
> should
> >>>> have been.
> >>>>
> >>>> For future reference, where would I find that kind of information?
> >>>> Would it be in the CORBA spec? The CDR encoding maybe?
> >>>>
> >>>> Cheers,
> >>>> - Matteo
> >>>>
> >>>>
> >>>> Darren Middleman wrote:
> >>>> > Hello Matteo,
> >>>> >
> >>>> > I think I've tracked down the cause of the problem.
> >>>> >
> >>>> > The reason this was happening was due to the way the conversion
> >>>> from the
> >>>> > Byte object to the Character object was being made.  Using a
> >>>> negative
> >>>> > byte
> >>>> > value to create a new character resulted in the value of the
> >>>> character
> >>>> > wrapping
> >>>> > within its range and causing a large character value.  (i.e. byte
> >>>> > value of
> >>>> > -128
> >>>> > resulted in a character with value 65408).  When this value was
> >>>> passed
> >>>> to
> >>>> > the
> >>>> > ORB, the stream checked to ensure that the value was not greater
> >>>> than
> >>>> 255
> >>>> > (the
> >>>> > OMG range for a character is 0 to 255) and since it was, threw
a
> >>>> > DATA_CONVERSION exception.
> >>>> >
> >>>> > I'm going to add something to the PrimitiveHandler so that the
> >>>> correct
> >>>> > conversion
> >>>> > from a byte value to a character value is performed.  This should
> >>>> > solve the
> >>>> > issues
> >>>> > you are seeing with the failing character test.
> >>>> >
> >>>> > Cheers,
> >>>> > Darren
> >>>> >
> >>>> >
> >>>> > On 2/12/07, Vescovi, Matteo <matteo.vescovi@iona.com> wrote:
> >>>> >>
> >>>> >> Hi,
> >>>> >> I am trying to fix some of the (currently disabled) idltowsdl
> corba
> >>>> >> binding type tests.
> >>>> >>
> >>>> >> The tests involve invoking an operation with the following
corba
> >>>> >> binding:
> >>>> >>     <wsdl:operation name="testChar">
> >>>> >>       <corba:operation name="testChar">
> >>>> >>         <corba:param mode="in" name="inChar"
> >>>> idltype="corba:char" />
> >>>> >>         <corba:param mode="inout" name="inoutChar"
> >>>> >> idltype="corba:char" />
> >>>> >>         <corba:param mode="out" name="outChar"
> >>>> idltype="corba:char" />
> >>>> >>         <corba:return name="return" idltype="corba:char"
/>
> >>>> >>       </corba:operation>
> >>>> >>
> >>>> >> In the client process, during the marshalling of the request,
a
> >>>> >> CORBA.DATA_CONVERSION exception is thrown when we attempt to
> >>>> write out
> >>>> >> the character.
> >>>> >>
> >>>> >> I think the problem lies in the fact that the type mapping
for a
> >>>> char
> >>>> is
> >>>> >> corba:char <-> xs:byte <-> java byte (or Byte).
> >>>> >> Class org.apache.yoko.bindings.corba.CorbaObjectWriter casts
the
> >>>> object
> >>>> >> to be marshalled to a Character in its writeChar method.
> >>>> >> Should this be cast to a Byte instead? Or should the Byte be
> >>>> >> appropriately converted to a Character before being written
out?
> >>>> >>
> >>>> >> I am not sure what the right approach to fix it is.
> >>>> >> Perhaps someone with a better understanding of the runtime
can
> >>>> help me
> >>>> >> here?
> >>>> >>
> >>>> >> Any help or pointers to relevant resources will be appreciated.
> >>>> >>
> >>>> >> To reproduce this failure, enable the test by commenting out
the
> >>>> >> testChar method in
> >>>> >>
> >>>> >>
> >>>>
> bindings/src/test/java/org/apache/yoko/bindings/corba/IdlToWsdlTypeTest.java
> >>>>
> >>>> >>
> >>>> >>
> >>>> >>
> >>>> >> Cheers,
> >>>> >> - Matteo
> >>>> >>
> >>>> >>
> >>>> >
> >>>>
> >>>>
> >>>
> >>
> >
>
>
> Index:
> bindings/src/test/java/org/apache/yoko/bindings/corba/IdlToWsdlTypeTest.java
> ===================================================================
> ---
> bindings/src/test/java/org/apache/yoko/bindings/corba/IdlToWsdlTypeTest.java        (revision
> 506453)
> +++
> bindings/src/test/java/org/apache/yoko/bindings/corba/IdlToWsdlTypeTest.java        (working
> copy)
> @@ -111,12 +111,10 @@
>
>
>      // following empty methods override real implementation until test
> failures are resolved
> -    public void testUnsignedShort() { }
>      public void testUnsignedLong() { }
> -    public void testChar() { }
>      public void testWchar() { }
>      public void testOctet () { }
> -    //public void testAny () { }
> +    public void testAny () { }
>      public void testString() { }
>      public void testWstring() { }
>      public void testStruct() { }
> Index:
> bindings/src/test/java/org/apache/yoko/bindings/corba/AbstractIdlToWsdlTypeTestClient.java
> ===================================================================
> ---
> bindings/src/test/java/org/apache/yoko/bindings/corba/AbstractIdlToWsdlTypeTestClient.java
 (revision
> 506453)
> +++
> bindings/src/test/java/org/apache/yoko/bindings/corba/AbstractIdlToWsdlTypeTestClient.java
 (working
> copy)
> @@ -171,16 +171,17 @@
>              {0, Integer.parseInt("16384")},
>              {0, Integer.parseInt("32767")},
>              {0, Integer.parseInt("32768")},
> -            {0, Integer.parseInt("65535")},
> -            {0, Integer.parseInt("65536")}
> +            {0, Integer.parseInt("65535")}
> +            //{0, Integer.parseInt("65536")}  // should fail here
>          };
> -
> +
>          for (int i = 0; i < valueSets.length; i++) {
>              int in = valueSets[i][0];
>              Holder<Integer> inoutOrig = new
> Holder<Integer>(valueSets[i][1]);
>              Holder<Integer> inout = new Holder<Integer>(valueSets[i][1]);
>              Holder<Integer> out = new Holder<Integer>();
>
> +            System.out.println("(short) inout: " + ((short)
> inout.value.shortValue()));
>              int ret = client.testUnsignedShort(in, inout, out);
>
>              assertEquals("testUnsignedShort(): Incorrect value for out
> param", inoutOrig.value, out.value);
> @@ -195,8 +196,11 @@
>          long valueSets[][] = {
>              {0, 0},
>              {0, 1},
> -            {0, MAX_UNSIGNED_LONG},
> -            {MAX_UNSIGNED_LONG, 0}
> +            {0, MAX_UNSIGNED_LONG / 2},
> +            {0, MAX_UNSIGNED_LONG * 5 / 8},
> +            {0, MAX_UNSIGNED_LONG * 3 / 4},
> +            {0, MAX_UNSIGNED_LONG - 1},
> +            {0, MAX_UNSIGNED_LONG}
>          };
>
>          for (int i = 0; i < valueSets.length; i++) {
> @@ -205,6 +209,7 @@
>              Holder<Long> inout = new Holder<Long>(valueSets[i][1]);
>              Holder<Long> out = new Holder<Long>();
>
> +            System.out.println("(long) inout: " + inout.value);
>              long ret = client.testUnsignedLong(in, inout, out);
>
>              assertEquals("testUnsignedLong(): Incorrect value for out
> param", inoutOrig.value, out.value);
> @@ -241,10 +246,12 @@
>              {0, 0},
>              {0, 1},
>              {1, 0},
> -            {Byte.MIN_VALUE, Byte.MAX_VALUE},
> -            {Byte.MAX_VALUE, Byte.MIN_VALUE}
> +            {0, Byte.MAX_VALUE},
> +            {'a', 'z'},
> +            {0, 127},
> +            {-128, 0}
>          };
> -
> +
>          for (int i = 0; i < valueSets.length; i++) {
>              byte in = valueSets[i][0];
>              Holder<Byte> inoutOrig = new Holder<Byte>(valueSets[i][1]);
> @@ -264,7 +271,7 @@
>              {"a", "b"},
>              {"b", "a"},
>              {"a", "z"},
> -            {"", "y"}
> +            {"0", "y"}
>          };
>
>          for (int i = 0; i < valueSets.length; i++) {
> Index:
> bindings/src/main/java/org/apache/yoko/bindings/corba/CorbaObjectReader.java
> ===================================================================
> ---
> bindings/src/main/java/org/apache/yoko/bindings/corba/CorbaObjectReader.java        (revision
> 506453)
> +++
> bindings/src/main/java/org/apache/yoko/bindings/corba/CorbaObjectReader.java        (working
> copy)
> @@ -190,7 +190,11 @@
>
>      public Integer readUShort() throws CorbaBindingException {
>          try {
> -            return new Integer(stream.read_ushort());
> +            Integer result = new Integer(stream.read_ushort());
> +            if (result < 0) {
> +                result = (result - Short.MIN_VALUE) - Short.MIN_VALUE;
> +            }
> +            return result;
>          } catch (org.omg.CORBA.MARSHAL ex) {
>              LOG.log(Level.SEVERE, "CorbaObjectReader: could not read
> unsigned short");
>              throw new CorbaBindingException("CorbaObjectReader:
> readUShort MARSHAL exception", ex);
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message