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: 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