A new user API enhancement proposal for Grizzly ProtocolParser

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

A new user API enhancement proposal for Grizzly ProtocolParser

CigarMan

A new user API enhancement proposal for Grizzly ProtocolParser

Reporting a bug

First and foremost, I would like to report a bug in Grizzly 1.7.2 with ParserProtocolFilter.java postExecute(Context context) method. When a ProtocolParser is invoked, on successfully parsing a token, the parser.releaseBuffer(); method gets called twice. To prevent this from happening, the following changes would have to be made to the ParserProtocolFilter.java postExecute(Context context) method:

 

    @Override

      public boolean postExecute(Context context) throws IOException {

            ProtocolParser parser = (ProtocolParser) context.getAttribute(ProtocolParser.PARSER);

            if (parser != null && parser.hasMoreBytesToParse()) {

                  // Need to say that we read successfully since bytes are left

                  context.setAttribute(ProtocolFilter.SUCCESSFUL_READ, Boolean.TRUE);

                  return true;

            }

 

            if (parser.isExpectingMoreData()) {

                  parser.releaseBuffer();

                  WorkerThread workerThread = (WorkerThread) Thread.currentThread();

                  SelectionKey key = context.getSelectionKey();

                  // Detach the current Thread data.

                  ThreadAttachment threadAttachment = workerThread.detach(true);

 

                  // Attach it to the SelectionKey so the it can be resumed latter.

                  key.attach(threadAttachment);

 

                  // Register to get more bytes.

                  context.getSelectorHandler().register(key, SelectionKey.OP_READ);

                  return false;

            }

 

**********

            if (!parser.hasMoreBytesToParse()) {

                  continousExecution = false;

            }

**********

            parser.releaseBuffer();

            return super.postExecute(context);

      }

 

Adding

 

            if (!parser.hasMoreBytesToParse()) {

                  continousExecution = false;

            }

 

Prevents the parser from being reinvoked if there are no more bytes to parse.

 

Running Grizzly’s unit test suite, I found no test cases that failed following the introduction of this fix.

 

Just let me know and I will post a bug ticket for it, I always prefer discussing issues on the mailing list first, this way, it lets the community in on issues which I encounter and it prevents bugs, which end up not being bugs, to be reported in tickets and waste developers time.

DefaultProtocolParser

Also, I would like to propose an addition to Grizzly to ease the development of ProtocolParser. With the current API provided, the buffer management when parsing is left to the API user. Defining a ProtocolParser is easy, making one that works in all cases is hard! Maybe I am slow, but it took me almost two full days to come up with one that works for all truncation cases. That is why I would like to propose DefaultProtocolParser.java. An abstract base class from which ProtocolParser could inherits and whose purpose is to handle all ByteBuffer management and only leaves the user to write the protected abstract int parse(T message, ByteBuffer readOnlyByteBuffer) method. It makes writing ProtocolParser a breeze!

 

 

So, without further ado, here’s DefaultProtocolParser:

 

 

/**

 * @author Simon Trudeau

 *

 */

public abstract class DefaultProtocolParser<T> implements ProtocolParser<T> {

      protected T message;

      protected ByteBuffer buffer;

      protected boolean expectingMoreData;

     

      public T getNextMessage() {

            return message;

      }

 

      public boolean hasMoreBytesToParse() {

            if(buffer == null)

            {

                  return false;

            }

            else

            {

                  return buffer.hasRemaining();

            }

      }

 

      public boolean hasNextMessage() {

            ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer();

            int tokenLength = parse(message, readOnlyByteBuffer);

            if(tokenLength > 0)

            {

                  /*

                   * We have found a message in the buffer

                   */

                  buffer.position(buffer.position() + tokenLength);

                  buffer.mark();

                  expectingMoreData = false;

                  return true;

            }

            else

            {

                  /*

                   * No message was found in the buffer

                   */

                  if(hasMoreBytesToParse())

                  {

                        expectingMoreData = true;

                  }

                  buffer.position(buffer.limit());

                  return false;

            }

      }

     

      /**

       * Parsing of the buffer takes place here.

       * @param message

       * @param readOnlyByteBuffer

       * @return The length of the token found, if no token is found, return 0.

       */

      protected abstract int parse(T message, ByteBuffer readOnlyByteBuffer);

     

 

      public boolean isExpectingMoreData() {

            return expectingMoreData;

      }

 

      /* (non-Javadoc)

       * @see com.sun.grizzly.ProtocolParser#releaseBuffer()

       * Restore the byteBuffer for writing.

       */

      public boolean releaseBuffer() {

            if(!hasMoreBytesToParse() && !isExpectingMoreData())

        {

                  /*

                   * All bytes in the buffer have been parsed into messages and no

                   * partial messages is left in unparsed in the buffer

                   */

            buffer.clear();

            return false;

        }

            if(isExpectingMoreData())

            {

                  buffer.reset();

                  buffer.compact();

                  return true;

            }

           

            return false;

      }

 

      /* (non-Javadoc)

       * @see com.sun.grizzly.ProtocolParser#startBuffer(java.nio.ByteBuffer)

       * Prepare the byteBuffer for reading

       */

      public void startBuffer(ByteBuffer bb) {

            buffer = bb;

            buffer.flip();

            buffer.mark();

      }

 

}

 

Writing a ProtocolParser is now as simple as writing:

 

public class MyProtocolPaser extends DefaultProtocolParser<String> {

 

      private static final Logger   LOG   = Logger.getLogger(Bt5070ProtocolParser.class);

     

      /*

     * Tokens to parse

     */

    private static final char CRs = 0x0d;

    private static final char LFs = 0x0a;

    private static final char EOSs = 0x00;

    private static final byte CR = 0x0d;

    private static final byte LF = 0x0a;

   

    private static final ProtocolToken PASS = new ProtocolToken("" + CRs + LFs + "PASS" + CRs + LFs + EOSs);

     

      @Override

      protected int parse(String message, ByteBuffer readOnlyByteBuffer) { 

        byte[] bufferContent = new byte[readOnlyByteBuffer.remaining()];

        readOnlyByteBuffer.get(bufferContent);

       

        String bufferContentString = new String(bufferContent);

        if(LOG.isTraceEnabled())

            {

                  LOG.trace("Attempting to parse : " + bufferContentString);

            }

        /*

             * 1) Parse the first 2 bytes of the buffer to see

             * if we are parsing a password negociation message or an

             * AT command

             * 1.1) If the first 2 bytes are 0x0d 0x0a

             * 1.1.1) Yes, check if buffer contains

             * 1.1.1.1) \r\nPASS\r\n\0

             * 1.1.1.5) If it doesn't contain any of the above, wait for more bytes because we are waiting for one of the above

             * 1.1.2) No, check the first 4 bytes to obtain the message length

             * 1.1.2.1) If byteBuffer content is smaller than the message length, wait for more bytes because there's got to be more!

             * 1.1.2.2) If byteBuffer content is bigger than the message length, return content from position 4 to message length

             */

       

        /*

         * Validates that the buffer contains at least 2 bytes so we can proceed to input validation

         */

        if(bufferContent.length < 2)

        {

            /*

             * We are expecting more data

             */

            return 0;

        }

      

        if(CR == bufferContent[0]) // 1.1)

        {

            if(LF == bufferContent[1])

            {

                  // 1.1.1)

                  if(bufferContentString.contains(PASS.toString()))

                  {

                        if(LOG.isTraceEnabled())

                        {

                              LOG.trace("Found: PASS");

                        }

                    message = PASS.toString();

                    return PASS.toBytes().length;

                  }

                  else

                  {

                        if(LOG.isTraceEnabled())

                        {

                              LOG.trace("Did not found any reconizable token");

                        }

                        return 0;

                  }                

            }

            else

            {

                  return 0;

            }

        }

        else if(bufferContent.length > 8)

        {

            // 1.1.2)

            IntBuffer bufferAsInt = readOnlyByteBuffer.asIntBuffer();

            int msgLength = bufferAsInt.get(0);

           

            // 1.1.2.1)

            if(bufferContent.length < msgLength)

            {

                  return 0;

            }

            else // 1.1.2.2)

            {

                  message = new String(bufferContent, 0, msgLength);

                  return msgLength;

            }

        }

        else

        {

            return 0;

        }

   

      }

}

 

The above has been successfully been tested for truncated packets with the following input:

 

  1. \r\nPASS\r\n\0
  2. \r\nPA and SS\r\n\0
  3. \r\nPA and SS\r\n\0\r\nPASS\r\n\0
  4. \r\nPASS\r\n\0\r\nPA and SS\r\n\0\r\nPASS\r\n\0
  5. \r\nPASS\r\n\0\r\nPASS\r\n\0
  6. \r\nPASS\r\n\0\r\nPASS\r\n\0\r\nPASS\r\n\0

 

Should you feel that I have left out some test cases, please let me know.

Conclusion:

I hope you will enjoy the new API class which I propose and I hope it’s going to make it into the next official Grizzly Release.

 

Let me know if you want me to fill up a request for enhancement with my proposal.

 

On a side note to the community, I would recommend making use of a real parser and grammar such as ANTLR for the parse method implementation if you are having anything more serious to parse than parsing a few tokens like in my example.

 

P.S.: I am sure this mailing list post would have fitted nicely into a blog… I’ve never blogged so is there a free one (something serious, maybe programming related) which you would recommend?

 

 

 

Simon

Reply | Threaded
Open this post in threaded view
|

Re: A new user API enhancement proposal for Grizzly ProtocolParser

Jeanfrancois Arcand-2
Hi Simon,

First, thanks for the feedback!!

Simon Trudeau wrote:

>
>   *A new user API enhancement proposal for Grizzly ProtocolParser*
>
>
>     */Reporting a bug/*
>
> First and foremost, I would like to report a bug in Grizzly 1.7.2 with
> ParserProtocolFilter.java postExecute(Context context) method. When a
> ProtocolParser is invoked, on successfully parsing a token, the
> parser.releaseBuffer(); method gets called twice. To prevent this from
> happening, the following changes would have to be made to the
> ParserProtocolFilter.java postExecute(Context context) method:
>
>  
>
>     @Override
>
>       *public* *boolean* postExecute(Context context) *throws* IOException {
>
>             ProtocolParser parser = (ProtocolParser)
> context.getAttribute(ProtocolParser./PARSER/);
>
>             *if* (parser != *null* && parser.hasMoreBytesToParse()) {
>
>                   // Need to say that we read successfully since bytes
> are left
>
>                   context.setAttribute(ProtocolFilter./SUCCESSFUL_READ/,
> Boolean./TRUE/);
>
>                   *return* *true*;
>
>             }
>
>  
>
>             *if* (parser.isExpectingMoreData()) {
>
>                   parser.releaseBuffer();
>
>                   WorkerThread workerThread = (WorkerThread)
> Thread./currentThread/();
>
>                   SelectionKey key = context.getSelectionKey();
>
>                   // Detach the current Thread data.
>
>                   ThreadAttachment threadAttachment =
> workerThread.detach(*true*);
>
>  
>
>                   // Attach it to the SelectionKey so the it can be
> resumed latter.
>
>                   key.attach(threadAttachment);
>
>  
>
>                   // Register to get more bytes.
>
>                   context.getSelectorHandler().register(key,
> SelectionKey./OP_READ/);
>
>                   *return* *false*;
>
>             }
>
>  
>
> **********
>
>             *if* (!parser.hasMoreBytesToParse()) {
>
>                   continousExecution = *false*;
>
>             }
>
> **********
>
>             parser.releaseBuffer();
>
>             *return* *super*.postExecute(context);
>
>       }
>
>  
>
> Adding
>
>  
>
>             *if* (!parser.hasMoreBytesToParse()) {
>
>                   continousExecution = *false*;
>
>             }
>

Looks good. Can you file an issue here:

https://grizzly.dev.java.net/issues/

and attach a diff -u of your patch? Will make quite simple for me to fix
the problem.

>  
>
> Prevents the parser from being reinvoked if there are no more bytes to
> parse.
>
>  
>
> Running Grizzly’s unit test suite, I found no test cases that failed
> following the introduction of this fix.

Great!! The fix will be easy :-)


>
>  
>
> Just let me know and I will post a bug ticket for it, I always prefer
> discussing issues on the mailing list first, this way, it lets the
> community in on issues which I encounter and it prevents bugs, which end
> up not being bugs, to be reported in tickets and waste developers time.

+1 This is a really good way of communicating! Seems we have added the
regression recently when we turned continuousExecution = true:

https://grizzly.dev.java.net/issues/show_bug.cgi?id=57

>
>
>     */DefaultProtocolParser/*
>
> Also, I would like to propose an addition to Grizzly to ease the
> development of ProtocolParser. With the current API provided, the buffer
> management when parsing is left to the API user. Defining a
> ProtocolParser is easy, making one that works in all cases is hard!

Yes buffer management is something missing in Grizzly...we kept talking
about it, but never had a chance to work on it.

> Maybe I am slow, but it took me almost two full days to come up with one
> that works for all truncation cases.

No you aren't. We must improve :-)

That is why I would like to propose

> DefaultProtocolParser.java. An abstract base class from which
> ProtocolParser could inherits and whose purpose is to handle all
> ByteBuffer management and only leaves the user to write the protected
> abstract int parse(T message, ByteBuffer readOnlyByteBuffer) method. It
> makes writing ProtocolParser a breeze!
>
>  
>
>  
>
> So, without further ado, here’s DefaultProtocolParser:
>
>  
>
>  
>
> /**
>
>  * *@author* Simon Trudeau
>
>  *
>
>  */
>
> *public* *abstract* *class* DefaultProtocolParser<T> *implements*
> ProtocolParser<T> {
>
>       *protected* T message;
>
>       *protected* ByteBuffer buffer;
>
>       *protected* *boolean* expectingMoreData;
>
>      
>
>       *public* T getNextMessage() {
>
>             *return* message;
>
>       }
>
>  
>
>       *public* *boolean* hasMoreBytesToParse() {
>
>             *if*(buffer == *null*)
>
>             {
>
>                   *return* *false*;
>
>             }
>
>             *else*
>
>             {
>
>                   *return* buffer.hasRemaining();
>
>             }
>
>       }
>
>  
>
>       *public* *boolean* hasNextMessage() {
>
>             ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer();
>
>             *int* tokenLength = parse(message, readOnlyByteBuffer);
>
>             *if*(tokenLength > 0)
>
>             {
>
>                   /*
>
>                    * We have found a message in the buffer
>
>                    */
>
>                   buffer.position(buffer.position() + tokenLength);
>
>                   buffer.mark();
>
>                   expectingMoreData = *false*;
>
>                   *return* *true*;
>
>             }
>
>             *else*
>
>             {
>
>                   /*
>
>                    * No message was found in the buffer
>
>                    */
>
>                   *if*(hasMoreBytesToParse())
>
>                   {
>
>                         expectingMoreData = *true*;
>
>                   }
>
>                   buffer.position(buffer.limit());
>
>                   *return* *false*;
>
>             }
>
>       }
>
>      
>
>       /**
>
>        * Parsing of the buffer takes place here.
>
>        * *@param* message
>
>        * *@param* readOnlyByteBuffer
>
>        * *@return* The length of the token found, if no token is found,
> return 0.
>
>        */
>
>       *protected* *abstract* *int* parse(T message, ByteBuffer
> readOnlyByteBuffer);
>
>      
>
>  
>
>       *public* *boolean* isExpectingMoreData() {
>
>             *return* expectingMoreData;
>
>       }
>
>  
>
>       /* (non-Javadoc)
>
>        * @see com.sun.grizzly.ProtocolParser#releaseBuffer()
>
>        * Restore the byteBuffer for writing.
>
>        */
>
>       *public* *boolean* releaseBuffer() {
>
>             *if*(!hasMoreBytesToParse() && !isExpectingMoreData())
>
>         {
>
>                   /*
>
>                    * All bytes in the buffer have been parsed into
> messages and no
>
>                    * partial messages is left in unparsed in the buffer
>
>                    */
>
>             buffer.clear();
>
>             *return* *false*;
>
>         }
>
>             *if*(isExpectingMoreData())
>
>             {
>
>                   buffer.reset();
>
>                   buffer.compact();
>
>                   *return* *true*;
>
>             }
>
>            
>
>             *return* *false*;
>
>       }
>
>  
>
>       /* (non-Javadoc)
>
>        * @see
> com.sun.grizzly.ProtocolParser#startBuffer(java.nio.ByteBuffer)
>
>        * Prepare the byteBuffer for reading
>
>        */
>
>       *public* *void* startBuffer(ByteBuffer bb) {
>
>             buffer = bb;
>
>             buffer.flip();
>
>             buffer.mark();
>
>       }
>
>  
>
> }
>
>  
>
> Writing a ProtocolParser is now as simple as writing:
>
>  
>
> *public* *class* MyProtocolPaser *extends* DefaultProtocolParser<String> {
>
>  
>
>       *private* *static* *final* Logger   /LOG/   =
> Logger./getLogger/(Bt5070ProtocolParser.*class*);
>
>      
>
>       /*
>
>      * Tokens to parse
>
>      */
>
>     *private* *static* *final* *char* /CRs/ = 0x0d;
>
>     *private* *static* *final* *char* /LFs/ = 0x0a;
>
>     *private* *static* *final* *char* /EOSs/ = 0x00;
>
>     *private* *static* *final* *byte* /CR/ = 0x0d;
>
>     *private* *static* *final* *byte* /LF/ = 0x0a;
>
>    
>
>     *private* *static* *final* ProtocolToken /PASS/ = *new*
> ProtocolToken("" + /CRs/ + /LFs/ + "PASS" + /CRs/ + /LFs/ + /EOSs/);
>
>      
>
>       @Override
>
>       *protected* *int* parse(String message, ByteBuffer
> readOnlyByteBuffer) {
>
>         *byte*[] bufferContent = *new*
> *byte*[readOnlyByteBuffer.remaining()];
>
>         readOnlyByteBuffer.get(bufferContent);
>
>        
>
>         String bufferContentString = *new* String(bufferContent);
>
>         *if*(/LOG/.isTraceEnabled())
>
>             {
>
>                   /LOG/.trace("Attempting to parse : " +
> bufferContentString);
>
>             }
>
>         /*
>
>              * 1) Parse the first 2 bytes of the buffer to see
>
>              * if we are parsing a password negociation message or an
>
>              * AT command
>
>              * 1.1) If the first 2 bytes are 0x0d 0x0a
>
>              * 1.1.1) Yes, check if buffer contains
>
>              * 1.1.1.1) \r\nPASS\r\n\0
>
>              * 1.1.1.5) If it doesn't contain any of the above, wait for
> more bytes because we are waiting for one of the above
>
>              * 1.1.2) No, check the first 4 bytes to obtain the message
> length
>
>              * 1.1.2.1) If byteBuffer content is smaller than the
> message length, wait for more bytes because there's got to be more!
>
>              * 1.1.2.2) If byteBuffer content is bigger than the message
> length, return content from position 4 to message length
>
>              */
>
>        
>
>         /*
>
>          * Validates that the buffer contains at least 2 bytes so we can
> proceed to input validation
>
>          */
>
>         *if*(bufferContent.length < 2)
>
>         {
>
>             /*
>
>              * We are expecting more data
>
>              */
>
>             *return* 0;
>
>         }
>
>      
>
>         *if*(/CR/ == bufferContent[0]) // 1.1)
>
>         {
>
>             *if*(/LF/ == bufferContent[1])
>
>             {
>
>                   // 1.1.1)
>
>                   *if*(bufferContentString.contains(/PASS/.toString()))
>
>                   {
>
>                         *if*(/LOG/.isTraceEnabled())
>
>                         {
>
>                               /LOG/.trace("Found: PASS");
>
>                         }
>
>                     message = /PASS/.toString();
>
>                     *return* /PASS/.toBytes().length;
>
>                   }
>
>                   *else*
>
>                   {
>
>                         *if*(/LOG/.isTraceEnabled())
>
>                         {
>
>                               /LOG/.trace("Did not found any reconizable
> token");
>
>                         }
>
>                         *return* 0;
>
>                   }                
>
>             }
>
>             *else*
>
>             {
>
>                   *return* 0;
>
>             }
>
>         }
>
>         *else* *if*(bufferContent.length > 8)
>
>         {
>
>             // 1.1.2)
>
>             IntBuffer bufferAsInt = readOnlyByteBuffer.asIntBuffer();
>
>             *int* msgLength = bufferAsInt.get(0);
>
>            
>
>             // 1.1.2.1)
>
>             *if*(bufferContent.length < msgLength)
>
>             {
>
>                   *return* 0;
>
>             }
>
>             *else* // 1.1.2.2)
>
>             {
>
>                   message = *new* String(bufferContent, 0, msgLength);
>
>                   *return* msgLength;
>
>             }
>
>         }
>
>         *else*
>
>         {
>
>             *return* 0;
>
>         }
>
>    
>
>       }
>
> }
>
>  
>
> The above has been successfully been tested for truncated packets with
> the following input:
>
>  
>
>    1. \r\nPASS\r\n\0
>    2. \r\nPA *and* SS\r\n\0
>    3. \r\nPA *and* SS\r\n\0\r\nPASS\r\n\0
>    4. \r\nPASS\r\n\0\r\nPA *and* SS\r\n\0\r\nPASS\r\n\0
>    5. \r\nPASS\r\n\0\r\nPASS\r\n\0
>    6. \r\nPASS\r\n\0\r\nPASS\r\n\0\r\nPASS\r\n\0
>
>  
>
> Should you feel that I have left out some test cases, please let me know.
>
>
>     */Conclusion:/*
>
> I hope you will enjoy the new API class which I propose and I hope it’s
> going to make it into the next official Grizzly Release.

+1 Can you file an enhancement and attach your stuff?



>
>  
>
> Let me know if you want me to fill up a request for enhancement with my
> proposal.

yes yes yes!

>
>  
>
> On a side note to the community, I would recommend making use of a real
> parser and grammar such as ANTLR for the parse method implementation if
> you are having anything more serious to parse than parsing a few tokens
> like in my example.

I agree. But with your code, we might see more contribution and one of
them might be using ANTLR. I think adding your code make things easier,
we just need to document it properly!

>
>  
>
> P.S.: I am sure this mailing list post would have fitted nicely into a
> blog… I’ve never blogged so is there a free one (something serious,
> maybe programming related) which you would recommend?

java.net is one that I'm using, but there is also blogger.com.

Thanks!

-- Jeanfrancois



>
>  
>
>  
>
>  
>
> Simon
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: Re: A new user API enhancement proposal for Grizzly ProtocolParser

CigarMan

I just filled a ticket for the ParserProtocolFilter bug:

https://grizzly.dev.java.net/issues/show_bug.cgi?id=91

I have cut and pasted a diff of the modification although I don't know
if it's ok since I am a windows/gui user and don't make much use of
command line/text commands :.)

Let me know if you were expecting something different.


Simon

-----Original Message-----
From: [hidden email] [mailto:[hidden email]]
Sent: March-10-08 11:29 AM
To: [hidden email]
Subject: Re: A new user API enhancement proposal for Grizzly
ProtocolParser

Hi Simon,

First, thanks for the feedback!!

Simon Trudeau wrote:
>
>   *A new user API enhancement proposal for Grizzly ProtocolParser*
>
>
>     */Reporting a bug/*
>
> First and foremost, I would like to report a bug in Grizzly 1.7.2 with

> ParserProtocolFilter.java postExecute(Context context) method. When a
> ProtocolParser is invoked, on successfully parsing a token, the
> parser.releaseBuffer(); method gets called twice. To prevent this from

> happening, the following changes would have to be made to the
> ParserProtocolFilter.java postExecute(Context context) method:
>
>  
>
>     @Override
>
>       *public* *boolean* postExecute(Context context) *throws*
IOException {
>
>             ProtocolParser parser = (ProtocolParser)
> context.getAttribute(ProtocolParser./PARSER/);
>
>             *if* (parser != *null* && parser.hasMoreBytesToParse()) {
>
>                   // Need to say that we read successfully since bytes

> are left
>
>
context.setAttribute(ProtocolFilter./SUCCESSFUL_READ/,

> Boolean./TRUE/);
>
>                   *return* *true*;
>
>             }
>
>  
>
>             *if* (parser.isExpectingMoreData()) {
>
>                   parser.releaseBuffer();
>
>                   WorkerThread workerThread = (WorkerThread)
> Thread./currentThread/();
>
>                   SelectionKey key = context.getSelectionKey();
>
>                   // Detach the current Thread data.
>
>                   ThreadAttachment threadAttachment =
> workerThread.detach(*true*);
>
>  
>
>                   // Attach it to the SelectionKey so the it can be
> resumed latter.
>
>                   key.attach(threadAttachment);
>
>  
>
>                   // Register to get more bytes.
>
>                   context.getSelectorHandler().register(key,
> SelectionKey./OP_READ/);
>
>                   *return* *false*;
>
>             }
>
>  
>
> **********
>
>             *if* (!parser.hasMoreBytesToParse()) {
>
>                   continousExecution = *false*;
>
>             }
>
> **********
>
>             parser.releaseBuffer();
>
>             *return* *super*.postExecute(context);
>
>       }
>
>  
>
> Adding
>
>  
>
>             *if* (!parser.hasMoreBytesToParse()) {
>
>                   continousExecution = *false*;
>
>             }
>

Looks good. Can you file an issue here:

https://grizzly.dev.java.net/issues/

and attach a diff -u of your patch? Will make quite simple for me to fix

the problem.

>  
>
> Prevents the parser from being reinvoked if there are no more bytes to

> parse.
>
>  
>
> Running Grizzly's unit test suite, I found no test cases that failed
> following the introduction of this fix.

Great!! The fix will be easy :-)


>
>  
>
> Just let me know and I will post a bug ticket for it, I always prefer
> discussing issues on the mailing list first, this way, it lets the
> community in on issues which I encounter and it prevents bugs, which
end
> up not being bugs, to be reported in tickets and waste developers
time.

+1 This is a really good way of communicating! Seems we have added the
regression recently when we turned continuousExecution = true:

https://grizzly.dev.java.net/issues/show_bug.cgi?id=57

>
>
>     */DefaultProtocolParser/*
>
> Also, I would like to propose an addition to Grizzly to ease the
> development of ProtocolParser. With the current API provided, the
buffer
> management when parsing is left to the API user. Defining a
> ProtocolParser is easy, making one that works in all cases is hard!

Yes buffer management is something missing in Grizzly...we kept talking
about it, but never had a chance to work on it.

> Maybe I am slow, but it took me almost two full days to come up with
one
> that works for all truncation cases.

No you aren't. We must improve :-)

That is why I would like to propose
> DefaultProtocolParser.java. An abstract base class from which
> ProtocolParser could inherits and whose purpose is to handle all
> ByteBuffer management and only leaves the user to write the protected
> abstract int parse(T message, ByteBuffer readOnlyByteBuffer) method.
It

> makes writing ProtocolParser a breeze!
>
>  
>
>  
>
> So, without further ado, here's DefaultProtocolParser:
>
>  
>
>  
>
> /**
>
>  * *@author* Simon Trudeau
>
>  *
>
>  */
>
> *public* *abstract* *class* DefaultProtocolParser<T> *implements*
> ProtocolParser<T> {
>
>       *protected* T message;
>
>       *protected* ByteBuffer buffer;
>
>       *protected* *boolean* expectingMoreData;
>
>      
>
>       *public* T getNextMessage() {
>
>             *return* message;
>
>       }
>
>  
>
>       *public* *boolean* hasMoreBytesToParse() {
>
>             *if*(buffer == *null*)
>
>             {
>
>                   *return* *false*;
>
>             }
>
>             *else*
>
>             {
>
>                   *return* buffer.hasRemaining();
>
>             }
>
>       }
>
>  
>
>       *public* *boolean* hasNextMessage() {
>
>             ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer();
>
>             *int* tokenLength = parse(message, readOnlyByteBuffer);
>
>             *if*(tokenLength > 0)
>
>             {
>
>                   /*
>
>                    * We have found a message in the buffer
>
>                    */
>
>                   buffer.position(buffer.position() + tokenLength);
>
>                   buffer.mark();
>
>                   expectingMoreData = *false*;
>
>                   *return* *true*;
>
>             }
>
>             *else*
>
>             {
>
>                   /*
>
>                    * No message was found in the buffer
>
>                    */
>
>                   *if*(hasMoreBytesToParse())
>
>                   {
>
>                         expectingMoreData = *true*;
>
>                   }
>
>                   buffer.position(buffer.limit());
>
>                   *return* *false*;
>
>             }
>
>       }
>
>      
>
>       /**
>
>        * Parsing of the buffer takes place here.
>
>        * *@param* message
>
>        * *@param* readOnlyByteBuffer
>
>        * *@return* The length of the token found, if no token is
found,

> return 0.
>
>        */
>
>       *protected* *abstract* *int* parse(T message, ByteBuffer
> readOnlyByteBuffer);
>
>      
>
>  
>
>       *public* *boolean* isExpectingMoreData() {
>
>             *return* expectingMoreData;
>
>       }
>
>  
>
>       /* (non-Javadoc)
>
>        * @see com.sun.grizzly.ProtocolParser#releaseBuffer()
>
>        * Restore the byteBuffer for writing.
>
>        */
>
>       *public* *boolean* releaseBuffer() {
>
>             *if*(!hasMoreBytesToParse() && !isExpectingMoreData())
>
>         {
>
>                   /*
>
>                    * All bytes in the buffer have been parsed into
> messages and no
>
>                    * partial messages is left in unparsed in the
buffer

>
>                    */
>
>             buffer.clear();
>
>             *return* *false*;
>
>         }
>
>             *if*(isExpectingMoreData())
>
>             {
>
>                   buffer.reset();
>
>                   buffer.compact();
>
>                   *return* *true*;
>
>             }
>
>            
>
>             *return* *false*;
>
>       }
>
>  
>
>       /* (non-Javadoc)
>
>        * @see
> com.sun.grizzly.ProtocolParser#startBuffer(java.nio.ByteBuffer)
>
>        * Prepare the byteBuffer for reading
>
>        */
>
>       *public* *void* startBuffer(ByteBuffer bb) {
>
>             buffer = bb;
>
>             buffer.flip();
>
>             buffer.mark();
>
>       }
>
>  
>
> }
>
>  
>
> Writing a ProtocolParser is now as simple as writing:
>
>  
>
> *public* *class* MyProtocolPaser *extends*
DefaultProtocolParser<String> {

>
>  
>
>       *private* *static* *final* Logger   /LOG/   =
> Logger./getLogger/(Bt5070ProtocolParser.*class*);
>
>      
>
>       /*
>
>      * Tokens to parse
>
>      */
>
>     *private* *static* *final* *char* /CRs/ = 0x0d;
>
>     *private* *static* *final* *char* /LFs/ = 0x0a;
>
>     *private* *static* *final* *char* /EOSs/ = 0x00;
>
>     *private* *static* *final* *byte* /CR/ = 0x0d;
>
>     *private* *static* *final* *byte* /LF/ = 0x0a;
>
>    
>
>     *private* *static* *final* ProtocolToken /PASS/ = *new*
> ProtocolToken("" + /CRs/ + /LFs/ + "PASS" + /CRs/ + /LFs/ + /EOSs/);
>
>      
>
>       @Override
>
>       *protected* *int* parse(String message, ByteBuffer
> readOnlyByteBuffer) {
>
>         *byte*[] bufferContent = *new*
> *byte*[readOnlyByteBuffer.remaining()];
>
>         readOnlyByteBuffer.get(bufferContent);
>
>        
>
>         String bufferContentString = *new* String(bufferContent);
>
>         *if*(/LOG/.isTraceEnabled())
>
>             {
>
>                   /LOG/.trace("Attempting to parse : " +
> bufferContentString);
>
>             }
>
>         /*
>
>              * 1) Parse the first 2 bytes of the buffer to see
>
>              * if we are parsing a password negociation message or an
>
>              * AT command
>
>              * 1.1) If the first 2 bytes are 0x0d 0x0a
>
>              * 1.1.1) Yes, check if buffer contains
>
>              * 1.1.1.1) \r\nPASS\r\n\0
>
>              * 1.1.1.5) If it doesn't contain any of the above, wait
for
> more bytes because we are waiting for one of the above
>
>              * 1.1.2) No, check the first 4 bytes to obtain the
message
> length
>
>              * 1.1.2.1) If byteBuffer content is smaller than the
> message length, wait for more bytes because there's got to be more!
>
>              * 1.1.2.2) If byteBuffer content is bigger than the
message
> length, return content from position 4 to message length
>
>              */
>
>        
>
>         /*
>
>          * Validates that the buffer contains at least 2 bytes so we
can

> proceed to input validation
>
>          */
>
>         *if*(bufferContent.length < 2)
>
>         {
>
>             /*
>
>              * We are expecting more data
>
>              */
>
>             *return* 0;
>
>         }
>
>      
>
>         *if*(/CR/ == bufferContent[0]) // 1.1)
>
>         {
>
>             *if*(/LF/ == bufferContent[1])
>
>             {
>
>                   // 1.1.1)
>
>
*if*(bufferContentString.contains(/PASS/.toString()))

>
>                   {
>
>                         *if*(/LOG/.isTraceEnabled())
>
>                         {
>
>                               /LOG/.trace("Found: PASS");
>
>                         }
>
>                     message = /PASS/.toString();
>
>                     *return* /PASS/.toBytes().length;
>
>                   }
>
>                   *else*
>
>                   {
>
>                         *if*(/LOG/.isTraceEnabled())
>
>                         {
>
>                               /LOG/.trace("Did not found any
reconizable

> token");
>
>                         }
>
>                         *return* 0;
>
>                   }                
>
>             }
>
>             *else*
>
>             {
>
>                   *return* 0;
>
>             }
>
>         }
>
>         *else* *if*(bufferContent.length > 8)
>
>         {
>
>             // 1.1.2)
>
>             IntBuffer bufferAsInt = readOnlyByteBuffer.asIntBuffer();
>
>             *int* msgLength = bufferAsInt.get(0);
>
>            
>
>             // 1.1.2.1)
>
>             *if*(bufferContent.length < msgLength)
>
>             {
>
>                   *return* 0;
>
>             }
>
>             *else* // 1.1.2.2)
>
>             {
>
>                   message = *new* String(bufferContent, 0, msgLength);
>
>                   *return* msgLength;
>
>             }
>
>         }
>
>         *else*
>
>         {
>
>             *return* 0;
>
>         }
>
>    
>
>       }
>
> }
>
>  
>
> The above has been successfully been tested for truncated packets with

> the following input:
>
>  
>
>    1. \r\nPASS\r\n\0
>    2. \r\nPA *and* SS\r\n\0
>    3. \r\nPA *and* SS\r\n\0\r\nPASS\r\n\0
>    4. \r\nPASS\r\n\0\r\nPA *and* SS\r\n\0\r\nPASS\r\n\0
>    5. \r\nPASS\r\n\0\r\nPASS\r\n\0
>    6. \r\nPASS\r\n\0\r\nPASS\r\n\0\r\nPASS\r\n\0
>
>  
>
> Should you feel that I have left out some test cases, please let me
know.
>
>
>     */Conclusion:/*
>
> I hope you will enjoy the new API class which I propose and I hope
it's
> going to make it into the next official Grizzly Release.

+1 Can you file an enhancement and attach your stuff?



>
>  
>
> Let me know if you want me to fill up a request for enhancement with
my
> proposal.

yes yes yes!

>
>  
>
> On a side note to the community, I would recommend making use of a
real
> parser and grammar such as ANTLR for the parse method implementation
if
> you are having anything more serious to parse than parsing a few
tokens
> like in my example.

I agree. But with your code, we might see more contribution and one of
them might be using ANTLR. I think adding your code make things easier,
we just need to document it properly!

>
>  
>
> P.S.: I am sure this mailing list post would have fitted nicely into a

> blog... I've never blogged so is there a free one (something serious,
> maybe programming related) which you would recommend?

java.net is one that I'm using, but there is also blogger.com.

Thanks!

-- Jeanfrancois



>
>  
>
>  
>
>  
>
> Simon
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: A new user API enhancement proposal for Grizzly ProtocolParser

Jeanfrancois Arcand-2
Salut

Simon Trudeau wrote:
> I just filled a ticket for the ParserProtocolFilter bug:
>
> https://grizzly.dev.java.net/issues/show_bug.cgi?id=91
>
> I have cut and pasted a diff of the modification although I don't know
> if it's ok since I am a windows/gui user and don't make much use of
> command line/text commands :.)
>
> Let me know if you were expecting something different.


This is perfect. Thanks!

-- Jeanfrancois


>
>
> Simon
>
> -----Original Message-----
> From: [hidden email] [mailto:[hidden email]]
> Sent: March-10-08 11:29 AM
> To: [hidden email]
> Subject: Re: A new user API enhancement proposal for Grizzly
> ProtocolParser
>
> Hi Simon,
>
> First, thanks for the feedback!!
>
> Simon Trudeau wrote:
>>   *A new user API enhancement proposal for Grizzly ProtocolParser*
>>
>>
>>     */Reporting a bug/*
>>
>> First and foremost, I would like to report a bug in Grizzly 1.7.2 with
>
>> ParserProtocolFilter.java postExecute(Context context) method. When a
>> ProtocolParser is invoked, on successfully parsing a token, the
>> parser.releaseBuffer(); method gets called twice. To prevent this from
>
>> happening, the following changes would have to be made to the
>> ParserProtocolFilter.java postExecute(Context context) method:
>>
>>  
>>
>>     @Override
>>
>>       *public* *boolean* postExecute(Context context) *throws*
> IOException {
>>             ProtocolParser parser = (ProtocolParser)
>> context.getAttribute(ProtocolParser./PARSER/);
>>
>>             *if* (parser != *null* && parser.hasMoreBytesToParse()) {
>>
>>                   // Need to say that we read successfully since bytes
>
>> are left
>>
>>
> context.setAttribute(ProtocolFilter./SUCCESSFUL_READ/,
>> Boolean./TRUE/);
>>
>>                   *return* *true*;
>>
>>             }
>>
>>  
>>
>>             *if* (parser.isExpectingMoreData()) {
>>
>>                   parser.releaseBuffer();
>>
>>                   WorkerThread workerThread = (WorkerThread)
>> Thread./currentThread/();
>>
>>                   SelectionKey key = context.getSelectionKey();
>>
>>                   // Detach the current Thread data.
>>
>>                   ThreadAttachment threadAttachment =
>> workerThread.detach(*true*);
>>
>>  
>>
>>                   // Attach it to the SelectionKey so the it can be
>> resumed latter.
>>
>>                   key.attach(threadAttachment);
>>
>>  
>>
>>                   // Register to get more bytes.
>>
>>                   context.getSelectorHandler().register(key,
>> SelectionKey./OP_READ/);
>>
>>                   *return* *false*;
>>
>>             }
>>
>>  
>>
>> **********
>>
>>             *if* (!parser.hasMoreBytesToParse()) {
>>
>>                   continousExecution = *false*;
>>
>>             }
>>
>> **********
>>
>>             parser.releaseBuffer();
>>
>>             *return* *super*.postExecute(context);
>>
>>       }
>>
>>  
>>
>> Adding
>>
>>  
>>
>>             *if* (!parser.hasMoreBytesToParse()) {
>>
>>                   continousExecution = *false*;
>>
>>             }
>>
>
> Looks good. Can you file an issue here:
>
> https://grizzly.dev.java.net/issues/
>
> and attach a diff -u of your patch? Will make quite simple for me to fix
>
> the problem.
>
>>  
>>
>> Prevents the parser from being reinvoked if there are no more bytes to
>
>> parse.
>>
>>  
>>
>> Running Grizzly's unit test suite, I found no test cases that failed
>> following the introduction of this fix.
>
> Great!! The fix will be easy :-)
>
>
>>  
>>
>> Just let me know and I will post a bug ticket for it, I always prefer
>> discussing issues on the mailing list first, this way, it lets the
>> community in on issues which I encounter and it prevents bugs, which
> end
>> up not being bugs, to be reported in tickets and waste developers
> time.
>
> +1 This is a really good way of communicating! Seems we have added the
> regression recently when we turned continuousExecution = true:
>
> https://grizzly.dev.java.net/issues/show_bug.cgi?id=57
>
>>
>>     */DefaultProtocolParser/*
>>
>> Also, I would like to propose an addition to Grizzly to ease the
>> development of ProtocolParser. With the current API provided, the
> buffer
>> management when parsing is left to the API user. Defining a
>> ProtocolParser is easy, making one that works in all cases is hard!
>
> Yes buffer management is something missing in Grizzly...we kept talking
> about it, but never had a chance to work on it.
>
>> Maybe I am slow, but it took me almost two full days to come up with
> one
>> that works for all truncation cases.
>
> No you aren't. We must improve :-)
>
> That is why I would like to propose
>> DefaultProtocolParser.java. An abstract base class from which
>> ProtocolParser could inherits and whose purpose is to handle all
>> ByteBuffer management and only leaves the user to write the protected
>> abstract int parse(T message, ByteBuffer readOnlyByteBuffer) method.
> It
>> makes writing ProtocolParser a breeze!
>>
>>  
>>
>>  
>>
>> So, without further ado, here's DefaultProtocolParser:
>>
>>  
>>
>>  
>>
>> /**
>>
>>  * *@author* Simon Trudeau
>>
>>  *
>>
>>  */
>>
>> *public* *abstract* *class* DefaultProtocolParser<T> *implements*
>> ProtocolParser<T> {
>>
>>       *protected* T message;
>>
>>       *protected* ByteBuffer buffer;
>>
>>       *protected* *boolean* expectingMoreData;
>>
>>      
>>
>>       *public* T getNextMessage() {
>>
>>             *return* message;
>>
>>       }
>>
>>  
>>
>>       *public* *boolean* hasMoreBytesToParse() {
>>
>>             *if*(buffer == *null*)
>>
>>             {
>>
>>                   *return* *false*;
>>
>>             }
>>
>>             *else*
>>
>>             {
>>
>>                   *return* buffer.hasRemaining();
>>
>>             }
>>
>>       }
>>
>>  
>>
>>       *public* *boolean* hasNextMessage() {
>>
>>             ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer();
>>
>>             *int* tokenLength = parse(message, readOnlyByteBuffer);
>>
>>             *if*(tokenLength > 0)
>>
>>             {
>>
>>                   /*
>>
>>                    * We have found a message in the buffer
>>
>>                    */
>>
>>                   buffer.position(buffer.position() + tokenLength);
>>
>>                   buffer.mark();
>>
>>                   expectingMoreData = *false*;
>>
>>                   *return* *true*;
>>
>>             }
>>
>>             *else*
>>
>>             {
>>
>>                   /*
>>
>>                    * No message was found in the buffer
>>
>>                    */
>>
>>                   *if*(hasMoreBytesToParse())
>>
>>                   {
>>
>>                         expectingMoreData = *true*;
>>
>>                   }
>>
>>                   buffer.position(buffer.limit());
>>
>>                   *return* *false*;
>>
>>             }
>>
>>       }
>>
>>      
>>
>>       /**
>>
>>        * Parsing of the buffer takes place here.
>>
>>        * *@param* message
>>
>>        * *@param* readOnlyByteBuffer
>>
>>        * *@return* The length of the token found, if no token is
> found,
>> return 0.
>>
>>        */
>>
>>       *protected* *abstract* *int* parse(T message, ByteBuffer
>> readOnlyByteBuffer);
>>
>>      
>>
>>  
>>
>>       *public* *boolean* isExpectingMoreData() {
>>
>>             *return* expectingMoreData;
>>
>>       }
>>
>>  
>>
>>       /* (non-Javadoc)
>>
>>        * @see com.sun.grizzly.ProtocolParser#releaseBuffer()
>>
>>        * Restore the byteBuffer for writing.
>>
>>        */
>>
>>       *public* *boolean* releaseBuffer() {
>>
>>             *if*(!hasMoreBytesToParse() && !isExpectingMoreData())
>>
>>         {
>>
>>                   /*
>>
>>                    * All bytes in the buffer have been parsed into
>> messages and no
>>
>>                    * partial messages is left in unparsed in the
> buffer
>>                    */
>>
>>             buffer.clear();
>>
>>             *return* *false*;
>>
>>         }
>>
>>             *if*(isExpectingMoreData())
>>
>>             {
>>
>>                   buffer.reset();
>>
>>                   buffer.compact();
>>
>>                   *return* *true*;
>>
>>             }
>>
>>            
>>
>>             *return* *false*;
>>
>>       }
>>
>>  
>>
>>       /* (non-Javadoc)
>>
>>        * @see
>> com.sun.grizzly.ProtocolParser#startBuffer(java.nio.ByteBuffer)
>>
>>        * Prepare the byteBuffer for reading
>>
>>        */
>>
>>       *public* *void* startBuffer(ByteBuffer bb) {
>>
>>             buffer = bb;
>>
>>             buffer.flip();
>>
>>             buffer.mark();
>>
>>       }
>>
>>  
>>
>> }
>>
>>  
>>
>> Writing a ProtocolParser is now as simple as writing:
>>
>>  
>>
>> *public* *class* MyProtocolPaser *extends*
> DefaultProtocolParser<String> {
>>  
>>
>>       *private* *static* *final* Logger   /LOG/   =
>> Logger./getLogger/(Bt5070ProtocolParser.*class*);
>>
>>      
>>
>>       /*
>>
>>      * Tokens to parse
>>
>>      */
>>
>>     *private* *static* *final* *char* /CRs/ = 0x0d;
>>
>>     *private* *static* *final* *char* /LFs/ = 0x0a;
>>
>>     *private* *static* *final* *char* /EOSs/ = 0x00;
>>
>>     *private* *static* *final* *byte* /CR/ = 0x0d;
>>
>>     *private* *static* *final* *byte* /LF/ = 0x0a;
>>
>>    
>>
>>     *private* *static* *final* ProtocolToken /PASS/ = *new*
>> ProtocolToken("" + /CRs/ + /LFs/ + "PASS" + /CRs/ + /LFs/ + /EOSs/);
>>
>>      
>>
>>       @Override
>>
>>       *protected* *int* parse(String message, ByteBuffer
>> readOnlyByteBuffer) {
>>
>>         *byte*[] bufferContent = *new*
>> *byte*[readOnlyByteBuffer.remaining()];
>>
>>         readOnlyByteBuffer.get(bufferContent);
>>
>>        
>>
>>         String bufferContentString = *new* String(bufferContent);
>>
>>         *if*(/LOG/.isTraceEnabled())
>>
>>             {
>>
>>                   /LOG/.trace("Attempting to parse : " +
>> bufferContentString);
>>
>>             }
>>
>>         /*
>>
>>              * 1) Parse the first 2 bytes of the buffer to see
>>
>>              * if we are parsing a password negociation message or an
>>
>>              * AT command
>>
>>              * 1.1) If the first 2 bytes are 0x0d 0x0a
>>
>>              * 1.1.1) Yes, check if buffer contains
>>
>>              * 1.1.1.1) \r\nPASS\r\n\0
>>
>>              * 1.1.1.5) If it doesn't contain any of the above, wait
> for
>> more bytes because we are waiting for one of the above
>>
>>              * 1.1.2) No, check the first 4 bytes to obtain the
> message
>> length
>>
>>              * 1.1.2.1) If byteBuffer content is smaller than the
>> message length, wait for more bytes because there's got to be more!
>>
>>              * 1.1.2.2) If byteBuffer content is bigger than the
> message
>> length, return content from position 4 to message length
>>
>>              */
>>
>>        
>>
>>         /*
>>
>>          * Validates that the buffer contains at least 2 bytes so we
> can
>> proceed to input validation
>>
>>          */
>>
>>         *if*(bufferContent.length < 2)
>>
>>         {
>>
>>             /*
>>
>>              * We are expecting more data
>>
>>              */
>>
>>             *return* 0;
>>
>>         }
>>
>>      
>>
>>         *if*(/CR/ == bufferContent[0]) // 1.1)
>>
>>         {
>>
>>             *if*(/LF/ == bufferContent[1])
>>
>>             {
>>
>>                   // 1.1.1)
>>
>>
> *if*(bufferContentString.contains(/PASS/.toString()))
>>                   {
>>
>>                         *if*(/LOG/.isTraceEnabled())
>>
>>                         {
>>
>>                               /LOG/.trace("Found: PASS");
>>
>>                         }
>>
>>                     message = /PASS/.toString();
>>
>>                     *return* /PASS/.toBytes().length;
>>
>>                   }
>>
>>                   *else*
>>
>>                   {
>>
>>                         *if*(/LOG/.isTraceEnabled())
>>
>>                         {
>>
>>                               /LOG/.trace("Did not found any
> reconizable
>> token");
>>
>>                         }
>>
>>                         *return* 0;
>>
>>                   }                
>>
>>             }
>>
>>             *else*
>>
>>             {
>>
>>                   *return* 0;
>>
>>             }
>>
>>         }
>>
>>         *else* *if*(bufferContent.length > 8)
>>
>>         {
>>
>>             // 1.1.2)
>>
>>             IntBuffer bufferAsInt = readOnlyByteBuffer.asIntBuffer();
>>
>>             *int* msgLength = bufferAsInt.get(0);
>>
>>            
>>
>>             // 1.1.2.1)
>>
>>             *if*(bufferContent.length < msgLength)
>>
>>             {
>>
>>                   *return* 0;
>>
>>             }
>>
>>             *else* // 1.1.2.2)
>>
>>             {
>>
>>                   message = *new* String(bufferContent, 0, msgLength);
>>
>>                   *return* msgLength;
>>
>>             }
>>
>>         }
>>
>>         *else*
>>
>>         {
>>
>>             *return* 0;
>>
>>         }
>>
>>    
>>
>>       }
>>
>> }
>>
>>  
>>
>> The above has been successfully been tested for truncated packets with
>
>> the following input:
>>
>>  
>>
>>    1. \r\nPASS\r\n\0
>>    2. \r\nPA *and* SS\r\n\0
>>    3. \r\nPA *and* SS\r\n\0\r\nPASS\r\n\0
>>    4. \r\nPASS\r\n\0\r\nPA *and* SS\r\n\0\r\nPASS\r\n\0
>>    5. \r\nPASS\r\n\0\r\nPASS\r\n\0
>>    6. \r\nPASS\r\n\0\r\nPASS\r\n\0\r\nPASS\r\n\0
>>
>>  
>>
>> Should you feel that I have left out some test cases, please let me
> know.
>>
>>     */Conclusion:/*
>>
>> I hope you will enjoy the new API class which I propose and I hope
> it's
>> going to make it into the next official Grizzly Release.
>
> +1 Can you file an enhancement and attach your stuff?
>
>
>
>>  
>>
>> Let me know if you want me to fill up a request for enhancement with
> my
>> proposal.
>
> yes yes yes!
>
>>  
>>
>> On a side note to the community, I would recommend making use of a
> real
>> parser and grammar such as ANTLR for the parse method implementation
> if
>> you are having anything more serious to parse than parsing a few
> tokens
>> like in my example.
>
> I agree. But with your code, we might see more contribution and one of
> them might be using ANTLR. I think adding your code make things easier,
> we just need to document it properly!
>
>>  
>>
>> P.S.: I am sure this mailing list post would have fitted nicely into a
>
>> blog... I've never blogged so is there a free one (something serious,
>> maybe programming related) which you would recommend?
>
> java.net is one that I'm using, but there is also blogger.com.
>
> Thanks!
>
> -- Jeanfrancois
>
>
>
>>  
>>
>>  
>>
>>  
>>
>> Simon
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: A new user API enhancement proposal for Grizzly ProtocolParser

ash2k!
In reply to this post by CigarMan

CigarMan wrote
DefaultProtocolParser


Also, I would like to propose an addition to Grizzly to ease the
development of ProtocolParser. With the current API provided, the buffer
management when parsing is left to the API user. Defining a
ProtocolParser is easy, making one that works in all cases is hard!
Maybe I am slow, but it took me almost two full days to come up with one
that works for all truncation cases. That is why I would like to propose
DefaultProtocolParser.java. An abstract base class from which
ProtocolParser could inherits and whose purpose is to handle all
ByteBuffer management and only leaves the user to write the protected
abstract int parse(T message, ByteBuffer readOnlyByteBuffer) method. It
makes writing ProtocolParser a breeze!

 

 

So, without further ado, here's DefaultProtocolParser:

 

 

/**

 * @author Simon Trudeau

 *

 */

public abstract class DefaultProtocolParser<T> implements
ProtocolParser<T> {

      protected T message;

      protected ByteBuffer buffer;

      protected boolean expectingMoreData;

     

      public T getNextMessage() {

            return message;

      }

 

      public boolean hasMoreBytesToParse() {

            if(buffer == null)

            {

                  return false;

            }

            else

            {

                  return buffer.hasRemaining();

            }

      }

 

      public boolean hasNextMessage() {

            ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer();

            int tokenLength = parse(message, readOnlyByteBuffer);

            if(tokenLength > 0)

            {

                  /*

                   * We have found a message in the buffer

                   */

                  buffer.position(buffer.position() + tokenLength);

                  buffer.mark();

                  expectingMoreData = false;

                  return true;

            }

            else

            {

                  /*

                   * No message was found in the buffer

                   */

                  if(hasMoreBytesToParse())

                  {

                        expectingMoreData = true;

                  }

                  buffer.position(buffer.limit());

                  return false;

            }

      }

     

      /**

       * Parsing of the buffer takes place here.

       * @param message

       * @param readOnlyByteBuffer

       * @return The length of the token found, if no token is found,
return 0.

       */

      protected abstract int parse(T message, ByteBuffer
readOnlyByteBuffer);

     

 

      public boolean isExpectingMoreData() {

            return expectingMoreData;

      }

 

      /* (non-Javadoc)

       * @see com.sun.grizzly.ProtocolParser#releaseBuffer()

       * Restore the byteBuffer for writing.

       */

      public boolean releaseBuffer() {

            if(!hasMoreBytesToParse() && !isExpectingMoreData())

        {

                  /*

                   * All bytes in the buffer have been parsed into
messages and no

                   * partial messages is left in unparsed in the buffer

                   */

            buffer.clear();

            return false;

        }

            if(isExpectingMoreData())

            {

                  buffer.reset();

                  buffer.compact();

                  return true;

            }

           

            return false;

      }

 

      /* (non-Javadoc)

       * @see
com.sun.grizzly.ProtocolParser#startBuffer(java.nio.ByteBuffer)

       * Prepare the byteBuffer for reading

       */

      public void startBuffer(ByteBuffer bb) {

            buffer = bb;

            buffer.flip();

            buffer.mark();

      }

 

}

 

Writing a ProtocolParser is now as simple as writing:

 

public class MyProtocolPaser extends DefaultProtocolParser<String> {

 

      private static final Logger   LOG   =
Logger.getLogger(Bt5070ProtocolParser.class);

     

      /*

     * Tokens to parse

     */

    private static final char CRs = 0x0d;

    private static final char LFs = 0x0a;

    private static final char EOSs = 0x00;

    private static final byte CR = 0x0d;

    private static final byte LF = 0x0a;

   

    private static final ProtocolToken PASS = new ProtocolToken("" + CRs
+ LFs + "PASS" + CRs + LFs + EOSs);

     

      @Override

      protected int parse(String message, ByteBuffer readOnlyByteBuffer)
{  

        byte[] bufferContent = new byte[readOnlyByteBuffer.remaining()];

        readOnlyByteBuffer.get(bufferContent);

       

        String bufferContentString = new String(bufferContent);

        if(LOG.isTraceEnabled())

            {

                  LOG.trace("Attempting to parse : " +
bufferContentString);

            }

        /*

             * 1) Parse the first 2 bytes of the buffer to see

             * if we are parsing a password negociation message or an

             * AT command

             * 1.1) If the first 2 bytes are 0x0d 0x0a

             * 1.1.1) Yes, check if buffer contains

             * 1.1.1.1) \r\nPASS\r\n\0

             * 1.1.1.5) If it doesn't contain any of the above, wait for
more bytes because we are waiting for one of the above

             * 1.1.2) No, check the first 4 bytes to obtain the message
length

             * 1.1.2.1) If byteBuffer content is smaller than the
message length, wait for more bytes because there's got to be more!

             * 1.1.2.2) If byteBuffer content is bigger than the message
length, return content from position 4 to message length

             */

       

        /*

         * Validates that the buffer contains at least 2 bytes so we can
proceed to input validation

         */

        if(bufferContent.length < 2)

        {

            /*

             * We are expecting more data

             */

            return 0;

        }

       

        if(CR == bufferContent[0]) // 1.1)

        {

            if(LF == bufferContent[1])

            {

                  // 1.1.1)

                  if(bufferContentString.contains(PASS.toString()))

                  {

                        if(LOG.isTraceEnabled())

                        {

                              LOG.trace("Found: PASS");

                        }

                    message = PASS.toString();

                    return PASS.toBytes().length;

                  }

                  else

                  {

                        if(LOG.isTraceEnabled())

                        {

                              LOG.trace("Did not found any reconizable
token");

                        }

                        return 0;

                  }                

            }

            else

            {

                  return 0;

            }

        }

        else if(bufferContent.length > 8)

        {

            // 1.1.2)

            IntBuffer bufferAsInt = readOnlyByteBuffer.asIntBuffer();

            int msgLength = bufferAsInt.get(0);

           

            // 1.1.2.1)

            if(bufferContent.length < msgLength)

            {

                  return 0;

            }

            else // 1.1.2.2)

            {

                  message = new String(bufferContent, 0, msgLength);

                  return msgLength;

            }

        }

        else

        {

            return 0;

        }

   

      }

}

 

The above has been successfully been tested for truncated packets with
the following input:

 

1. \r\nPASS\r\n\0
2. \r\nPA and SS\r\n\0
3. \r\nPA and SS\r\n\0\r\nPASS\r\n\0
4. \r\nPASS\r\n\0\r\nPA and SS\r\n\0\r\nPASS\r\n\0
5. \r\nPASS\r\n\0\r\nPASS\r\n\0
6. \r\nPASS\r\n\0\r\nPASS\r\n\0\r\nPASS\r\n\0

 

Should you feel that I have left out some test cases, please let me
know.


Conclusion:


I hope you will enjoy the new API class which I propose and I hope it's
going to make it into the next official Grizzly Release.

 

Let me know if you want me to fill up a request for enhancement with my
proposal.

 
IMHO idei and code is good but dont forget to mention this code cant handle packets bigger than the (default) size of the bytebuffer - 8k.