[Edonkey-java-devel] Re: TODO: gui-core connection packet handling

[Tim <t....@ze...>]

On Friday 16 May 2003 20:51, Robin wrote:

Hi,

> I find a little time to work on the EJC, ran through the
> startup code, up to the serious things: the CoreController
> Class, the one that handle the communication between the GUI
> and the CLC.
>
> It seems that you are right, the communication and packet
> handling is somehow...bad.
> It's as you describe:

I had a quick look at CoreController.java in jc/src/com/mm/net/, and found 
basically only the following issue:

* how does the EJC behave if the packet size doesn't match the size read?

   (a) if read > packet size: not sure what happens. This case should not
        happen though.

   (b) if packet size > read: line 745 should show a message 
        "size is not match with actual read" (sic). The problem that I see is
        that the EJC might not read in those 'superfluous' bytes, nor will it
        skip those bytes. But maybe I'm just wrong :-)
        A simple 3 lines here could resolve quite a few problems (see below).


> [MetaInputStream stuff:]
> As you can see, the message is parsed in several times:
> First the Header, than the size (in 4 consecutive read), and
> then the message id, and after, the contents in several
> times.
> It seems clear that your advice and method is better, but I
> don't know Java enough (I'll look at this in detail later)
> to know if a single read on the socket would deliver a
> packet in "one shot" (I'm more experienced with C), I should
> investigate this.
> But whatever the method is, reading the header, than the
> size, than the payload should work in both case (reading a
> block, or reading byte after byte). I suspect a bug either
> in the CLC, either in the GUI (I suspect the GUI).

Maybe we should leave that stuff alone then for now, and rather fix the stuff 
as it is, that might be easier?



> I'd prefer to just correct a bug, or adjust a function that
> read a certain type of message than to rewrite the whole
> packet reading code (this could come in a second time).

yes, agreed. Just ignore my first mail to the list then :-)



> PS: look at what I find after activating the trace code:
> Size is not match with actual read: MessageID: 199 size: 123
> actual read: 108
> ?(-76)
> Retrieving
> http://sda.edonkey2000.com/ver/ver.php?ui=2&ver=1.2.0&core=1
> 0481
> (0)(0)(0)(28)(4)(3)(0)
> the parenthesis correspond to this code:
> MessageUtils.trace(new String("" + (char) header + "(" +
> header + ")"));
>
> We found a bug, I'll try to find it with more precision, and
> i'll let you know

aaah. The newer cores send an options message as one of the first messages 
when the GUI connects. So if the GUI fails here, everything else will be 
unlikely to work, as the EJG apparently does not re-sync the input stream to 
the next packet header.

Try inserting this code in CoreController.java from line 735 and see if that 
makes it work:

            if (!processed) {
                byte[] result = new byte[size];
                int read = 0;
                while (read < size)
                    read += in.read(result, read, size - read);
                MessageUtils.trace(new String(result) + "\n");
            }
            else {
                int end = in.getCount();
+              byte[] resttoignore = new byte[end-read+1];
+              while (read < size)
+                   read += in.read(resttoignore, read, size - read);
                if (end - start != size) {
                    MessageUtils.trace("Size is not match with actual read: 
MessageID: " + (256 + msgID) + " size: " + size + " actual read: " + (end - 
start) + "\n");
                }
            }
            in.setCount(0);

or similar (I've never coded java, so this might be plain wrong). What it's 
supposed to do is just read in the extra X bytes of the packet into a buffer 
and ignore them.
 


> > [snip]
>
> Huh, as I said, whatever it receives, if the CLC respects
> his own protocol (HEADER-SIZE-MSGID-PAYLOAD), there should
> be no "out of sync" situation. May I suspect the CLC not to
> respect this simple protocol ?
> the message: Size is not match with actual read: MessageID:
> 199 size: 123 actual read: 108
> seems to confirm the fact, isn't it ?

No, the CLC respects this protocol. The CLC will always produce packets where 
a header that says 'size=X' is followed by X bytes of data. Always :-)

In this case it's just that the options message (199) has been extended by me 
by a few bytes. So the core sends a header that says 'size=123', but the EJG 
only expects 108 bytes, because it doesn't know about my extensions yet. But 
instead of skipping the 15 bytes it doesn't know what to do with it just 
prints out a message and goes on parsing at those unexpected bytes, expecting 
a new packet header there. At least if I'm correct. And that would 
definitively be a bug in the Java client....

cheers
-Tim