FAQ
Hi all, I think it would be nice to implement zlib compression for
golang.org/x/crypto/ssh, but the ssh spec specifies quite clearly how to
flush the payload at the end of a packet. It requires what is essentially
the Z_PARTIAL_FLUSH method of zlib. Here’s the relevant part of the RFC,
as well as a good summary of the different flush methods.


http://tools.ietf.org/html/rfc4253#section-6.2

http://www.bolet.org/~pornin/deflate-flush-fr.html


As a side note, Z_PARTIAL_FLUSH was deprecated for a while, but is now
un-deprecated again (http://www.zlib.net/ChangeLog.txt).


This “partial flush” method isn’t implemented in compress/flate—the Writer
type has one method, Flush(), which corresponds to the Z_SYNC_FLUSH method
in zlib. (maybe because of the temporary deprecation?) So we can’t use
compress/flate to implement ssh compression correctly.


On the other hand, it seems that at least one SSH implementation
(http://www.lysator.liu.se/~nisse/lsh/) uses Z_SYNC_FLUSH, so apparently it
doesn’t cause problems in practice.


I think it still makes sense to conform to the ssh spec, but this means
adding partial flush functionality to flate.Writer. I can think of a few
potentially reasonable API shapes:


1) [backwards-incompatible] Drop the Flush method and replace it with
specific SyncFlush and PartialFlush methods.

2) [backwards-incompatible] Change the Flush method to take a parameter
specifying the type of flush.

3) [backwards-compatible] Add SyncFlush and PartialFlush methods, leaving
Flush as a sort of “recommended/default flush”, and suggesting that you use
the other flush methods if you care what kind you get.

4) [backwards-compatible] Simply add PartialFlush, just leaving Flush as
is. This is maybe the most likely option, as mentioned in the code review
for the original
Flush(): https://groups.google.com/forum/#!searchin/golang-dev/flate$20flush/golang-dev/XARip6_k42A/593ISDhrCV4J
  It does leave the names a bit inconsistent with zlib, however.


So, assuming that this is a reasonable thing to add, what should the API
look like?


Cheers,

Aaron


--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Search Discussions

  • Brad Fitzpatrick at Dec 14, 2014 at 7:17 am
    I think option 4.

    On Sun, Dec 7, 2014 at 10:11 AM, wrote:

    Hi all, I think it would be nice to implement zlib compression for
    golang.org/x/crypto/ssh, but the ssh spec specifies quite clearly how to
    flush the payload at the end of a packet. It requires what is essentially
    the Z_PARTIAL_FLUSH method of zlib. Here’s the relevant part of the RFC,
    as well as a good summary of the different flush methods.


    http://tools.ietf.org/html/rfc4253#section-6.2

    http://www.bolet.org/~pornin/deflate-flush-fr.html


    As a side note, Z_PARTIAL_FLUSH was deprecated for a while, but is now
    un-deprecated again (http://www.zlib.net/ChangeLog.txt).


    This “partial flush” method isn’t implemented in compress/flate—the Writer
    type has one method, Flush(), which corresponds to the Z_SYNC_FLUSH method
    in zlib. (maybe because of the temporary deprecation?) So we can’t use
    compress/flate to implement ssh compression correctly.


    On the other hand, it seems that at least one SSH implementation (
    http://www.lysator.liu.se/~nisse/lsh/) uses Z_SYNC_FLUSH, so apparently
    it doesn’t cause problems in practice.


    I think it still makes sense to conform to the ssh spec, but this means
    adding partial flush functionality to flate.Writer. I can think of a few
    potentially reasonable API shapes:


    1) [backwards-incompatible] Drop the Flush method and replace it with
    specific SyncFlush and PartialFlush methods.

    2) [backwards-incompatible] Change the Flush method to take a parameter
    specifying the type of flush.

    3) [backwards-compatible] Add SyncFlush and PartialFlush methods, leaving
    Flush as a sort of “recommended/default flush”, and suggesting that you use
    the other flush methods if you care what kind you get.

    4) [backwards-compatible] Simply add PartialFlush, just leaving Flush as
    is. This is maybe the most likely option, as mentioned in the code review
    for the original Flush():
    https://groups.google.com/forum/#!searchin/golang-dev/flate$20flush/golang-dev/XARip6_k42A/593ISDhrCV4J
    It does leave the names a bit inconsistent with zlib, however.


    So, assuming that this is a reasonable thing to add, what should the API
    look like?


    Cheers,

    Aaron


    --
    You received this message because you are subscribed to the Google Groups
    "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Minux at Dec 14, 2014 at 7:33 am
    Do we really need partial flush? Is there demonstrable need that sync flush
    won't fill? (I mean, if we use sync flush to flush ssh packets, will that
    cause
    real problems with existing ssh implementations?)

    If there is, then I also vote for option 4. compress/flate don't need to
    match
    zlib, and they doesn't match now. I think normally when an application needs
    flush, it needs sync flush, not partial flush, so it makes sense to have
    Flush()
    and then PartialFlush() for very specific needs.

    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 7, '14 at 6:48p
activeDec 14, '14 at 7:33a
posts3
users3
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase