Re: A new user API enhancement proposal for Grizzly ProtocolParser
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: Jeanfrancois.Arcand_at_Sun.COM [mailto:Jeanfrancois.Arcand_at_Sun.COM]
> Sent: March-10-08 11:29 AM
> To: users_at_grizzly.dev.java.net
> 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:
>>
>>
>>
>>
>>
>> /**
>>
>> * *_at_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.
>>
>> * *_at_param* message
>>
>> * *_at_param* readOnlyByteBuffer
>>
>> * *_at_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: users-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>