Bug 232530 - [MSN] Session and subclasses needs to handle whitespace and exceptions better
Summary: [MSN] Session and subclasses needs to handle whitespace and exceptions better
Status: RESOLVED FIXED
Alias: None
Product: ECF
Classification: RT
Component: ecf.protocols (show other bugs)
Version: 2.0.0   Edit
Hardware: All All
: P3 critical (vote)
Target Milestone: ---   Edit
Assignee: Remy Suen CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-05-16 12:09 EDT by Stoyan Boshev CLA
Modified: 2008-05-18 14:27 EDT (History)
2 users (show)

See Also:


Attachments
Patch for the processing of read messages bugs (3.30 KB, patch)
2008-05-17 04:51 EDT, Stoyan Boshev CLA
no flags Details | Diff
Revised patch against HEAD. (5.58 KB, patch)
2008-05-18 08:32 EDT, Remy Suen CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stoyan Boshev CLA 2008-05-16 12:09:46 EDT
I am using ECF 2.0.0.v20080514

I am seeing very frequently parsing problems in the ChatSession.read() method which lead to exception which causes the death of the Session$IdleThread that processes the received messages. If an unexpected Exception occurs I can no longer receive messages in the current session. So perhaps it is better to protect the Session$IdleThread  from any uncaught exceptions.

One of the cases that leads to the described behavior is this:
The original received message which is being processed by the ChatSession.read():

"MSG s_boshev@prosyst.bg s_boshev@prosyst.bg 129
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
X-MMS-IM-Format: FN=MS%20Shell%20Dlg; EF=; CO=0; CS=cc; PF=0

e de"

This leads to this exception:

Exception in thread "Thread-12" java.lang.StringIndexOutOfBoundsException: String index out of range: 5
	at java.lang.String.checkBounds(String.java:402)
	at java.lang.String.<init>(String.java:443)
	at org.eclipse.ecf.protocol.msn.ChatSession.read(ChatSession.java:295)
	at org.eclipse.ecf.protocol.msn.Session$IdleThread.run(Session.java:287)
Comment 1 Scott Lewis CLA 2008-05-16 13:00:47 EDT
Adding Remy.
Comment 2 Remy Suen CLA 2008-05-16 13:03:56 EDT
Putting in my bucket.
Comment 3 Stoyan Boshev CLA 2008-05-17 04:43:20 EDT
I dug more into the problem. It seems the problem occurs when a message that ends with one or more spaces is received. The read method trims the spaces and therefore the message that is being parsed contains less characters than expected.
Comment 4 Stoyan Boshev CLA 2008-05-17 04:51:20 EDT
Created attachment 100765 [details]
Patch for the processing of read messages bugs

The patch fixes the problems which are related to:

- processing of messaged that end with white spaces
- unexpected exception leads to stop of the processing of further received messages
- processing of messages that contain leading new lines like "\r\n\r\nsome message" (such messages were displayed like "")
Comment 5 Scott Lewis CLA 2008-05-17 18:08:33 EDT
Remy, if you can take a look at this and apply/test/release to HEAD before Mon 19 we'll get this fix into RC1.  

In any event thanks Stoyan for the contribution.

Comment 6 Remy Suen CLA 2008-05-18 08:32:40 EDT
Created attachment 100814 [details]
Revised patch against HEAD.

This new patch only "ignores" RuntimeExceptions and not all Throwables. I'm pretty sure we don't want to be ignoring java.lang.Errors. :)

Scott, can I get a +1 here?
Comment 7 Scott Lewis CLA 2008-05-18 11:11:41 EDT
(In reply to comment #6)
> Created an attachment (id=100814) [details]
> Revised patch against HEAD.
> 
> This new patch only "ignores" RuntimeExceptions and not all Throwables. I'm
> pretty sure we don't want to be ignoring java.lang.Errors. :)
> 
> Scott, can I get a +1 here?
> 

+1

Thanks for addressing quickly.
Comment 8 Remy Suen CLA 2008-05-18 11:44:11 EDT
Patch released to HEAD.

Thanks for the help, Stoyan!
Comment 9 Remy Suen CLA 2008-05-18 11:45:02 EDT
Whoops, forgot to resolve.
Comment 10 Scott Lewis CLA 2008-05-18 14:27:43 EDT
Added contributed keyword.