You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Spark Shen <sm...@gmail.com> on 2007/11/12 03:53:32 UTC
[classlib][security] HARMONY-5054 status was ([jira] Created: (HARMONY-5054) [ASN.1] BerInputStream will incorrectly resize buffer when the enveloped InputStream has lots of bytes)
Hi
Stepan, Sorry for the so late reply. Regis, Liang Jia & I was busy
developing ldap features and did not notice your comment. Please see my in
line comments.
<snip>
But I can not understand what wrong with the BerInputStream constructor - in
case of InputStream parameter the data is read from the beggining so offeset
has to be 0.
</snip>
I think the problem is the next() method invocation on line 132 of
constructor. In this method offset is updated. So when the program goes
through line 137, the offset
is no longer zero.
<snip>
A standalone test case would greatly help to understand what should be
fixed.
</snip>
The test case requires a ldap server, and I planned to provide a scenario
test and a adapter to BTI. But the effort was not trial as I expected.
<snip>
Could you provide first 10 bytes of incoming data and total size of ldap
server request?
</snip>
Wait a minute please, I am working on it. :-)
--
Spark Shen
China Software Development Lab, IBM
Re: [classlib][security] HARMONY-5054 status was ([jira] Created: (HARMONY-5054) [ASN.1] BerInputStream will incorrectly resize buffer when the enveloped InputStream has lots of bytes)
Posted by Spark Shen <sm...@gmail.com>.
2007/11/12, Spark Shen <sm...@gmail.com>:
>
> Hi
>
> Stepan, Sorry for the so late reply. Regis, Liang Jia & I was busy
> developing ldap features and did not notice your comment. Please see my in
> line comments.
>
> <snip>
> But I can not understand what wrong with the BerInputStream constructor -
> in case of InputStream parameter the data is read from the beggining so
> offeset has to be 0.
> </snip>
> I think the problem is the next() method invocation on line 132 of
> constructor. In this method offset is updated. So when the program goes
> through line 137, the offset
> is no longer zero.
>
> <snip>
> A standalone test case would greatly help to understand what should be
> fixed.
> </snip>
> The test case requires a ldap server, and I planned to provide a scenario
> test and a adapter to BTI. But the effort was not trial as I expected.
>
> <snip>
> Could you provide first 10 bytes of incoming data and total size of ldap
> server request?
> </snip>
> Wait a minute please, I am working on it. :-)
> --
> Spark Shen
> China Software Development Lab, IBM
The following test case will reveal the problem:
package test;
import java.io.ByteArrayInputStream;
import org.apache.harmony.security.asn1.ASN1OctetString;
public class ASN1Test {
public static void main(String[] args) {
try {
StringBuilder builder = new StringBuilder();
for (int i = 0; i < 17000; i++) {
builder.append("1");
}
ASN1OctetString octString = new ASN1OctetString();
byte[] bytes = null;
bytes = octString.encode(builder.toString().getBytes("UTF-8"));
Object st;
st = octString.decode(new ByteArrayInputStream(bytes));
String string = new String((byte[]) st);
System.out.println(string);
return;
} catch (Exception e1) {
e1.printStackTrace();
}
}
}
--
Spark Shen
China Software Development Lab, IBM
Re: [classlib][security] HARMONY-5054 status was ([jira] Created: (HARMONY-5054) [ASN.1] BerInputStream will incorrectly resize buffer when the enveloped InputStream has lots of bytes)
Posted by Stepan Mishura <st...@gmail.com>.
On 11/15/07, Spark Shen <sm...@gmail.com> wrote:
> 2007/11/15, Stepan Mishura <st...@gmail.com>:
> >
> > On 11/14/07, Stepan Mishura <st...@gmail.com> wrote:
> > <SNIP>
> > > > When decoding input from an inputStream retrieved from socket, the if
> > > > statement here is not adequate
> > > > to guarantee all the response bytes from server been collected.
> > > >
> > > > I doubt the off-line reproducer can unveal this bug. Correct if I am
> > wrong.
> > > >
> > >
> > > I got the idea and going to try to create off-line reproducer by
> > > implementing custom InputStream.
> >
> > Spark,
> >
> > Does the test below model correctly your case?
> >
> > byte[] encoding = { 0x04, 0x0F, 0x01, 0x02,
> > 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B,
> > 0x0C,
> > 0x0D, 0x0E, 0x0F };
> >
> > // custom input stream that doesn't return all data at once
> > ByteArrayInputStream in = new ByteArrayInputStream(encoding) {
> > public int read(byte[] b, int off, int len) {
> > if (len < 2) {
> > return super.read(b, off, len);
> > } else {
> > return super.read(b, off, len - 1);
> > }
> >
> > }
> > };
> >
> > BerInputStream berIn = new BerInputStream(in);
> > berIn.readContent();
>
>
> Oh, Yes, I think so.
>
Fixed at r595216. Please verify.
-Stepan.
Re: [classlib][security] HARMONY-5054 status was ([jira] Created: (HARMONY-5054) [ASN.1] BerInputStream will incorrectly resize buffer when the enveloped InputStream has lots of bytes)
Posted by Spark Shen <sm...@gmail.com>.
2007/11/15, Stepan Mishura <st...@gmail.com>:
>
> On 11/14/07, Stepan Mishura <st...@gmail.com> wrote:
> <SNIP>
> > > When decoding input from an inputStream retrieved from socket, the if
> > > statement here is not adequate
> > > to guarantee all the response bytes from server been collected.
> > >
> > > I doubt the off-line reproducer can unveal this bug. Correct if I am
> wrong.
> > >
> >
> > I got the idea and going to try to create off-line reproducer by
> > implementing custom InputStream.
>
> Spark,
>
> Does the test below model correctly your case?
>
> byte[] encoding = { 0x04, 0x0F, 0x01, 0x02,
> 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B,
> 0x0C,
> 0x0D, 0x0E, 0x0F };
>
> // custom input stream that doesn't return all data at once
> ByteArrayInputStream in = new ByteArrayInputStream(encoding) {
> public int read(byte[] b, int off, int len) {
> if (len < 2) {
> return super.read(b, off, len);
> } else {
> return super.read(b, off, len - 1);
> }
>
> }
> };
>
> BerInputStream berIn = new BerInputStream(in);
> berIn.readContent();
Oh, Yes, I think so.
Thanks,
> Stepan.
>
--
Spark Shen
China Software Development Lab, IBM
Re: [classlib][security] HARMONY-5054 status was ([jira] Created: (HARMONY-5054) [ASN.1] BerInputStream will incorrectly resize buffer when the enveloped InputStream has lots of bytes)
Posted by Stepan Mishura <st...@gmail.com>.
On 11/14/07, Stepan Mishura <st...@gmail.com> wrote:
<SNIP>
> > When decoding input from an inputStream retrieved from socket, the if
> > statement here is not adequate
> > to guarantee all the response bytes from server been collected.
> >
> > I doubt the off-line reproducer can unveal this bug. Correct if I am wrong.
> >
>
> I got the idea and going to try to create off-line reproducer by
> implementing custom InputStream.
Spark,
Does the test below model correctly your case?
byte[] encoding = { 0x04, 0x0F, 0x01, 0x02,
0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C,
0x0D, 0x0E, 0x0F };
// custom input stream that doesn't return all data at once
ByteArrayInputStream in = new ByteArrayInputStream(encoding) {
public int read(byte[] b, int off, int len) {
if (len < 2) {
return super.read(b, off, len);
} else {
return super.read(b, off, len - 1);
}
}
};
BerInputStream berIn = new BerInputStream(in);
berIn.readContent();
Thanks,
Stepan.
Re: [classlib][security] HARMONY-5054 status was ([jira] Created: (HARMONY-5054) [ASN.1] BerInputStream will incorrectly resize buffer when the enveloped InputStream has lots of bytes)
Posted by Stepan Mishura <st...@gmail.com>.
On 11/14/07, Spark Shen <sm...@gmail.com> wrote:
<SNIP>
>
> I checked your fix, and seems my second point did not convinced you. :-)
Quite opposite :-)
I think the issues you reported are different so I'm going to do separate fixes.
> 2. In method readContent, the if statement:
>
> if (in.read(buffer, offset, length) != length) {
> throw new ASN1Exception(Messages.getString("security.13C"));
> //$NON-NLS-1$
> }
> offset += length;
> is not enough to guarantee all the bytes are read into buffer. This can be
> fixed using a while loop:
>
> int numread = 0, oldoffset = offset;
> while ((numread = in.read(buffer, offset, length)) > 0) {
> offset += numread;
> length -= numread;
> if(length == 0) {
> break;
> }
> }
> length = offset - oldoffset;
> When decoding input from an inputStream retrieved from socket, the if
> statement here is not adequate
> to guarantee all the response bytes from server been collected.
>
> I doubt the off-line reproducer can unveal this bug. Correct if I am wrong.
>
I got the idea and going to try to create off-line reproducer by
implementing custom InputStream.
Thanks,
Stepan.
<SNIP>
Re: [classlib][security] HARMONY-5054 status was ([jira] Created: (HARMONY-5054) [ASN.1] BerInputStream will incorrectly resize buffer when the enveloped InputStream has lots of bytes)
Posted by Spark Shen <sm...@gmail.com>.
2007/11/12, Stepan Mishura <st...@gmail.com>:
>
> On 11/12/07, Spark Shen <sm...@gmail.com> wrote:
> > Hi
> >
> > Stepan, Sorry for the so late reply. Regis, Liang Jia & I was busy
> > developing ldap features and did not notice your comment. Please see my
> in
> > line comments.
> >
> > <snip>
> > But I can not understand what wrong with the BerInputStream constructor
> - in
> > case of InputStream parameter the data is read from the beggining so
> offeset
> > has to be 0.
> > </snip>
> > I think the problem is the next() method invocation on line 132 of
> > constructor. In this method offset is updated. So when the program goes
> > through line 137, the offset
> > is no longer zero.
> >
>
> Hi Spark,
>
> You are right. Thanks for the reproducer!
> I think this execution path has never been tested. I've fixed it at
> r594188.
>
> > <snip>
> > A standalone test case would greatly help to understand what should be
> > fixed.
> > </snip>
> > The test case requires a ldap server, and I planned to provide a
> scenario
> > test and a adapter to BTI. But the effort was not trial as I expected.
> >
>
> If you let me know how large ldap request is and its bytes values in
> the beginning then I can try to create a reproducer without ldap
> server.
The request is:
[48, -127, -105, 2, 1, 2, 99, 117, 4, 12, 99, 110, 61, 83, 117, 98, 115, 99,
104, 101, 109, 97, 10, 1, 0, 10, 1, 3, 2, 1, 0, 2, 1, 0, 1, 1, 0, -93, 24,
4, 11, 111, 98, 106, 101, 99, 116, 99, 108, 97, 115, 115, 4, 9, 115, 117,
98, 115, 99, 104, 101, 109, 97, 48, 60, 4, 13, 111, 98, 106, 101, 99, 116,
99, 108, 97, 115, 115, 101, 115, 4, 14, 97, 116, 116, 114, 105, 98, 117,
116, 101, 116, 121, 112, 101, 115, 4, 13, 109, 97, 116, 99, 104, 105, 110,
103, 114, 117, 108, 101, 115, 4, 12, 108, 100, 97, 112, 115, 121, 110, 116,
97, 120, 101, 115, -96, 27, 48, 25, 4, 23, 50, 46, 49, 54, 46, 56, 52, 48,
46, 49, 46, 49, 49, 51, 55, 51, 48, 46, 51, 46, 52, 46, 50]
The search is sent to an openldap server, against DN "cn=Subschema".
The search scope is object scope.
The required attributes are "objectclasses","attributetypes",
"matchingrules", "ldapsyntaxes" respectively.
I checked your fix, and seems my second point did not convinced you. :-)
2. In method readContent, the if statement:
if (in.read(buffer, offset, length) != length) {
throw new ASN1Exception(Messages.getString("security.13C"));
//$NON-NLS-1$
}
offset += length;
is not enough to guarantee all the bytes are read into buffer. This can be
fixed using a while loop:
int numread = 0, oldoffset = offset;
while ((numread = in.read(buffer, offset, length)) > 0) {
offset += numread;
length -= numread;
if(length == 0) {
break;
}
}
length = offset - oldoffset;
When decoding input from an inputStream retrieved from socket, the if
statement here is not adequate
to guarantee all the response bytes from server been collected.
I doubt the off-line reproducer can unveal this bug. Correct if I am wrong.
Thanks,
> Stepan.
>
> > <snip>
> > Could you provide first 10 bytes of incoming data and total size of ldap
> > server request?
> > </snip>
> > Wait a minute please, I am working on it. :-)
> > --
> > Spark Shen
> > China Software Development Lab, IBM
> >
>
--
Spark Shen
China Software Development Lab, IBM
Re: [classlib][security] HARMONY-5054 status was ([jira] Created: (HARMONY-5054) [ASN.1] BerInputStream will incorrectly resize buffer when the enveloped InputStream has lots of bytes)
Posted by Stepan Mishura <st...@gmail.com>.
On 11/12/07, Spark Shen <sm...@gmail.com> wrote:
> Hi
>
> Stepan, Sorry for the so late reply. Regis, Liang Jia & I was busy
> developing ldap features and did not notice your comment. Please see my in
> line comments.
>
> <snip>
> But I can not understand what wrong with the BerInputStream constructor - in
> case of InputStream parameter the data is read from the beggining so offeset
> has to be 0.
> </snip>
> I think the problem is the next() method invocation on line 132 of
> constructor. In this method offset is updated. So when the program goes
> through line 137, the offset
> is no longer zero.
>
Hi Spark,
You are right. Thanks for the reproducer!
I think this execution path has never been tested. I've fixed it at r594188.
> <snip>
> A standalone test case would greatly help to understand what should be
> fixed.
> </snip>
> The test case requires a ldap server, and I planned to provide a scenario
> test and a adapter to BTI. But the effort was not trial as I expected.
>
If you let me know how large ldap request is and its bytes values in
the beginning then I can try to create a reproducer without ldap
server.
Thanks,
Stepan.
> <snip>
> Could you provide first 10 bytes of incoming data and total size of ldap
> server request?
> </snip>
> Wait a minute please, I am working on it. :-)
> --
> Spark Shen
> China Software Development Lab, IBM
>