ZLIF support

Bug #783516 reported by iceman50
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
DC++
Fix Released
Wishlist
Unassigned

Bug Description

I have created a patch for adding ZLIF support in DC++ I tested it on FlipFlop's test flexhub and works

Related branches

Revision history for this message
iceman50 (bdcdevel) wrote :
Revision history for this message
poy (poy) wrote :

the patch is good as is. but i have a few questions:

1) what happens to ZLIB-GET now, do we keep using it for binary transfers or can it be merged with ZLIB-FULL? the doc seems to suggest, although vaguely, that the latter could be possible.

2) TLS encryption: how useful is it to compress data that will later be encrypted?

3) why does ADC need a ZOF message to mark the end of a compressed stream but NMDC doesn't?

4) error handling: at the moment, decompression errors are silently ignored. they may however occur, either because DC++ doesn't have enough memory to create the zlib decompressor, or during decompression when the Adler-32 checksum doesn't match the contents.
perhaps an STA should be sent to the hub, requesting it to send the messages it just sent in uncompressed form (this would require book-keeping from hubs that they may not want to do)? or a SUP with RMZLIF ("remove ZLIB-FULL support")? or, easier, just close the connection and reconnect without ZLIB-FULL support?
hubs will most likely compress the initial INF pack and i don't think it would be sane to continue a hub connection if these INFs couldn't be fetched.

Revision history for this message
iceman50 (bdcdevel) wrote :

well I would propose either keeping ZLIB-GET for compatibility with older clients (even though we could technically use ZLIF for everything would mean more work in the end for compatibility with older clients)

2) I can't technically speak about TLS until i can test it in a hub with TLS *hint hint* ADCH++ perhaps? =)

3) iirc it was an escaping issue with nmdc but i could be wrong

4) a STA message would be nice, and yeah that's something i noticed when i was seeing how the hub would send the compressed data without me ever decompressing ...

 To Hub: HSUP ADBAS0 ADBASE ADTIGR ADUCM0 ADBLO0 ADZLIF
 From Hub: ISTA 000 Sendingscompressedsmessage
 From Hub: IZON
 From Hub: xmP_n_0__+_$)__7__b_K-_8___F___e______̑_~:ѰOp__s_P__k8@_]/#__љ]_@_P]'d__0kkF__(o <_A_nB_'ZuGڳ!Z<x_ __{@__%__:_m_p_X_]&______
90_S_fĩY_f_X_5__z>
w__&_х_ۈ]__4__g6_7_____2_{2W_A_L_}_oӪZ_______U_K__ݭ__e__W_'_=<_+X_$_o__n_}?
E|_ISTA 000 Compressedsmessagessent.

which right there was about 150 users BINF's so you end up in a hub with no users BINF's which is quite annoying so a way to have some failsafe with that would be nice

Revision history for this message
iceman50 (bdcdevel) wrote :

ok reading up on ZPipe0 apparently we substitute ZOF instead of using an escaping defined by zlib (the way NMDC) i think using a zlib defined escaping would be a better choice then substituting ZOF for it

Revision history for this message
Jacek Sieka (arnetheduck) wrote :

1) zlib-get stays for now
2) tls is irrelevant really as this feature is orthogonal to tls - for the tls:s that compress, the hub owner can disable compression at the hub..
3) in nmdc compression is done until the zlib end of stream - the zof message is kind of redundant as the compression must end just after it but it's there in the spec so we have to use and support it
4) Errors = disconnect (if tcp failed to correctly transfer the bytes, we're screwed anyway...). It is kind of important that hubs flush their streams, but that's really a hub implementation issue

I'm applying the patch.

Changed in dcplusplus:
status: New → Fix Committed
importance: Undecided → Wishlist
Revision history for this message
iceman50 (bdcdevel) wrote :

I say (since there is no proper forum yet) that we should modify the specification to remove ZOF, will have to talk to Pretorian and perhaps host some sort of discussion in the Dev hub <adcs://hub.dcbase.org:16591> (for those who subscribe here but do not know the updated URL)

Revision history for this message
poy (poy) wrote :

Fixed in DC++ 0.790.

Changed in dcplusplus:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.