You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Emmanuel Lécharny <el...@gmail.com> on 2013/05/20 11:14:44 UTC
Dubious code in SslHelper...
Hi guys,
in SslHelper, we do have some code that unwrap the incoming data :
private void processUnwrap(AbstractIoSession session, ByteBuffer
inBuffer) throws SSLException {
// Blind guess : once uncompressed, the resulting buffer will be
3 times bigger
ByteBuffer appBuffer = ByteBuffer.allocate(inBuffer.limit() * 3);
SSLEngineResult result = unwrap(inBuffer, appBuffer);
<<------------------------ Here
switch (result.getStatus()) {
case OK:
// Ok, go through the chain now
appBuffer.flip();
session.processMessageReceived(appBuffer);
and the called method unwrap does some strange thing :
private SSLEngineResult unwrap(ByteBuffer inBuffer, ByteBuffer
appBuffer) throws SSLException {
// First work with either the new incoming buffer, or the
accumulating buffer
ByteBuffer tempBuffer = inBuffer;
// Loop until we have processed the entire incoming buffer,
// or until we have to stop
while (true) {
// Do the unwrapping
SSLEngineResult result = sslEngine.unwrap(tempBuffer,
appBuffer);
switch (result.getStatus()) {
...
case BUFFER_OVERFLOW:
// We have to increase the appBuffer size. In any case
// we aren't processing an handshake here. Read again.
appBuffer = ByteBuffer.allocate(appBuffer.capacity() +
4096); <<--------------- ???
}
}
}
I don't see how possibly the appBuffer can be used after having been
extended. In fact, I do think that the appBuffer is already big enough
(we allocate 3 times the needed size), but it's also a mistake to do so
: we should get back the result of the unwrap method and increase the
buffer size accordingly instead.
Thoughts ?
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: Dubious code in SslHelper...
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 5/21/13 1:32 PM, Jeff MAURY a écrit :
> For me, SSL code in trunk is just garbage and we should merge
> ssl-experiment into trunk
Ahhhh...
Ok, just do that then !
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: Dubious code in SslHelper...
Posted by Jeff MAURY <je...@jeffmaury.com>.
For me, SSL code in trunk is just garbage and we should merge
ssl-experiment into trunk
Regards
Jeff
On Tue, May 21, 2013 at 1:27 PM, Emmanuel Lécharny <el...@gmail.com>wrote:
> Le 5/21/13 1:19 PM, Jeff MAURY a écrit :
> > Which branch are you talking about ? trunk or ssl-experiment ?
>
> Trunk.
>
> ssl-experiment is a dead branch.
>
>
> --
> Regards,
> Cordialement,
> Emmanuel Lécharny
> www.iktek.com
>
>
--
Jeff MAURY
"Legacy code" often differs from its suggested alternative by actually
working and scaling.
- Bjarne Stroustrup
http://www.jeffmaury.com
http://riadiscuss.jeffmaury.com
http://www.twitter.com/jeffmaury
Re: Dubious code in SslHelper...
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 5/21/13 1:19 PM, Jeff MAURY a écrit :
> Which branch are you talking about ? trunk or ssl-experiment ?
Trunk.
ssl-experiment is a dead branch.
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: Dubious code in SslHelper...
Posted by Jeff MAURY <je...@jeffmaury.com>.
Which branch are you talking about ? trunk or ssl-experiment ?
Jeff
On Mon, May 20, 2013 at 11:14 AM, Emmanuel Lécharny <el...@gmail.com>wrote:
> Hi guys,
>
> in SslHelper, we do have some code that unwrap the incoming data :
>
> private void processUnwrap(AbstractIoSession session, ByteBuffer
> inBuffer) throws SSLException {
> // Blind guess : once uncompressed, the resulting buffer will be
> 3 times bigger
> ByteBuffer appBuffer = ByteBuffer.allocate(inBuffer.limit() * 3);
> SSLEngineResult result = unwrap(inBuffer, appBuffer);
> <<------------------------ Here
>
> switch (result.getStatus()) {
> case OK:
> // Ok, go through the chain now
> appBuffer.flip();
> session.processMessageReceived(appBuffer);
>
> and the called method unwrap does some strange thing :
>
> private SSLEngineResult unwrap(ByteBuffer inBuffer, ByteBuffer
> appBuffer) throws SSLException {
> // First work with either the new incoming buffer, or the
> accumulating buffer
> ByteBuffer tempBuffer = inBuffer;
>
> // Loop until we have processed the entire incoming buffer,
> // or until we have to stop
> while (true) {
> // Do the unwrapping
> SSLEngineResult result = sslEngine.unwrap(tempBuffer,
> appBuffer);
>
> switch (result.getStatus()) {
> ...
>
> case BUFFER_OVERFLOW:
> // We have to increase the appBuffer size. In any case
> // we aren't processing an handshake here. Read again.
> appBuffer = ByteBuffer.allocate(appBuffer.capacity() +
> 4096); <<--------------- ???
> }
> }
> }
>
> I don't see how possibly the appBuffer can be used after having been
> extended. In fact, I do think that the appBuffer is already big enough
> (we allocate 3 times the needed size), but it's also a mistake to do so
> : we should get back the result of the unwrap method and increase the
> buffer size accordingly instead.
>
> Thoughts ?
>
> --
> Regards,
> Cordialement,
> Emmanuel Lécharny
> www.iktek.com
>
>
--
Jeff MAURY
"Legacy code" often differs from its suggested alternative by actually
working and scaling.
- Bjarne Stroustrup
http://www.jeffmaury.com
http://riadiscuss.jeffmaury.com
http://www.twitter.com/jeffmaury