Config. variable max_xmpp_buffer_size not respected in current HEAD

[dhruvbird]

Probably _on_data() & _on_stanza() should update a counter and maintain the buffer under max_xmpp_buffer_size.

[satyamshekhar]

Done. This actually limits the xmpp stanza size. We should also put in a limit for the buffer size - size of pending_stanzas.

[dhruvbird]

Thanks :)

On Tue, Apr 3, 2012 at 1:47 AM, satyamshekhar
reply@reply.github.com
wrote:

Done. This actually limits the xmpp stanza size. We should also put in a limit for the buffer size - size of pending_stanzas.

---

Reply to this email directly or view it on GitHub:
#23 (comment)

   -Dhruv Matani.
http://dhruvbird.com/

"What's the simplest thing that could possibly work?"
-- Ward Cunningham

[dhruvbird]

Just wondering - suppose the limit on the packet size is 100 bytes and
the server sends 200 bytes in one go, won't that trigger this check?
Post-parsing though, each stanza might actually be <= 100 bytes in size?

On Tue, Apr 3, 2012 at 9:16 AM, Dhruv Matani dhruvbird@gmail.com wrote:

Thanks :)

On Tue, Apr 3, 2012 at 1:47 AM, satyamshekhar
reply@reply.github.com
wrote:

Done. This actually limits the xmpp stanza size. We should also put in a limit for the buffer size - size of pending_stanzas.

---

Reply to this email directly or view it on GitHub:
#23 (comment)

   -Dhruv Matani.
http://dhruvbird.com/

"What's the simplest thing that could possibly work?"
-- Ward Cunningham

   -Dhruv Matani.
http://dhruvbird.com/

"What's the simplest thing that could possibly work?"
-- Ward Cunningham

[satyamshekhar]

Yup, this check will not terminate that the stream in that case..

[dhruvbird]

I am guessing that the check you have implemented is for buffer size limit.

On Tue, Apr 3, 2012 at 2:56 PM, satyamshekhar
reply@reply.github.com
wrote:

Yup, this check will not terminate that the stream in that case..

---

Reply to this email directly or view it on GitHub:
#23 (comment)

   -Dhruv Matani.
http://dhruvbird.com/

"What's the simplest thing that could possibly work?"
-- Ward Cunningham

[satyamshekhar]

No, this only checks the stanza size. _current_stanza_size is reset in function _on_stanza. To implement buffer size limit we will have to limit the size of pending_stanzas right?

[dhruvbird]

What I meant to say is that the current check does more than just the
stanza size - it disallows valid stanzas < the limit if the server
decided to push multiple stanzas at one go right?

So in essence, it is limiting the amount of buffer space we allocate
for incoming packets for a certain stream.

On Tue, Apr 3, 2012 at 3:05 PM, satyamshekhar
reply@reply.github.com
wrote:

No, this only checks the stanza size. _current_stanza_size is reset in function _on_stanza. To implement buffer size limit we will have to limit the size of pending_stanzas right?

---

Reply to this email directly or view it on GitHub:
#23 (comment)

   -Dhruv Matani.
http://dhruvbird.com/

"What's the simplest thing that could possibly work?"
-- Ward Cunningham

[satyamshekhar]

Oh, now I get what you are saying.. Actually, not necessarily.. this depends on the network/node.js.. it might emit the data into diff chunks.. The size of the chunks is not decided by the remote server, is it?

[dhruvbird]

On Tue, Apr 3, 2012 at 3:17 PM, satyamshekhar
reply@reply.github.com
wrote:

Oh, now I get what you are saying.. Actually, not necessarily.. this depends on the network/node.js.. it might emit the data into diff chunks.. The size of the chunks is not decided by the remote server, is it?

Exactly! So node itself might actually accumulate multiple packets and
push them out right?

Actually, your last observation makes me thing that it is not entirely
possible to reliably limit the stanza size (or the buffer size) unless
it has been completely parsed or the last chunk of the stanza remains.
Consider when the stanza limit is 100 bytes and we have received 90
bytes. The server (or node) then sends 100 bytes, 5 of which end the
current stanza and the remaining start a new one. Our check fails
here.

However, we can have a config variable which limits the amount of data
we are willing to receive and hold and process on behalf of the server
(this is essentially the buffer size). Again, this would be best
effort.

---

Reply to this email directly or view it on GitHub:
#23 (comment)

   -Dhruv Matani.
http://dhruvbird.com/

"What's the simplest thing that could possibly work?"
-- Ward Cunningham

[satyamshekhar]

Exactly! So node itself might actually accumulate multiple packets and
push them out right?

It can accumulate packets in case current tick takes time.. What we can do is move the check after we write the incoming data to the parser.. that will limit the stanza size right?

We can have another check that limits the chunk size.. Like you already suggested..

[sonnyp]

Sorry, wrong issue number :-/

[dhruvbird]

@satyamshekhar @astro has pushed a feature to node-expat that allows one to query the number of bytes read in the stream till now. I've tried to fix the stanza limit restriction. Please could you review it for correctness. Caveat: It is possible that stanzas > the limit are accepted at times, but never will stanzas <= the limit be rejected. The logic is somewhat like a bloom filter.

[satyamshekhar]

looks good. (y)