Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of per message deflate extension #574

Closed
marci4 opened this issue Oct 13, 2017 · 11 comments · Fixed by #866
Closed

Implementation of per message deflate extension #574

marci4 opened this issue Oct 13, 2017 · 11 comments · Fixed by #866

Comments

@marci4
Copy link
Collaborator

marci4 commented Oct 13, 2017

Planned for a future version!

https://tools.ietf.org/html/rfc7692

@marci4 marci4 modified the milestones: Release 1.3.6, Planned for future Release Oct 13, 2017
@marci4 marci4 added this to To do in Development via automation Nov 9, 2017
@marci4 marci4 removed this from the Planned for future Release milestone Nov 9, 2017
@haruntuncay
Copy link
Collaborator

Hey @marci4, I want to struggle with this issue but it seems a bit dated. I just wanted to confirm whether it is still expected/needed or not.

Also, I may have to request your opinion on some design issues sometimes if that's okay ? 😅

@marci4
Copy link
Collaborator Author

marci4 commented Feb 12, 2019

hello @haruntuncay,

thank you for your interest in this issue!

Yes this is still somewhat planned but I simple does not have the time for this.

Of course, you can send me a mail or ping me in github whenever you want :) Always happy to help.

Best regards,
Marcel

@haruntuncay
Copy link
Collaborator

haruntuncay commented Feb 27, 2019

Hi @marci4,
I have been doing some research on LZ77, Deflate and their implementation and now I have some free-time and I want to start on the implementation but I would like to hear your opinion on something;

I have found this generic LZ77 implementation by Apache but I don't know about license issues and I also implemented LZ77 myself using the same hash chaining technique that is described in section 4 of RFC-1951 Deflate Algorithm with a circular-sliding window.

Now, I don't know which one would be more suitable for the project and I wanted to hear your idea on which should be chosen. I personally would like to use my own implementation because I think it would be easier to maintain and has no license issues, btw I will share my impl on github later today.

Do you think it is more suitable to choose ready-built implementation or implement it separately and use that ? (both for LZ77 and Deflate)

EDIT

I have decided to NOT to re-invent the wheel and use the LZ77 implementation provided under Apache license. Please give me a heads-up if you think that would not be appropriate for the project.

@NoahAndrews
Copy link
Contributor

Why would the Apache license be a problem?

@haruntuncay
Copy link
Collaborator

haruntuncay commented Feb 28, 2019

Hey @marci4,
Do you know about java.util.zip.Deflater ? Based on this stackoverflow question, he says that java.util.zip.Deflater can produce rfc 1951 compliant output. Since I don't know java.util.zip.Deflater very much, I wanted to ask you about it. Do you think it can get the deflate compression part done or should I write a deflate implementation around this LZ77 implementation ?

@marci4
Copy link
Collaborator Author

marci4 commented Feb 28, 2019

TL;DR

I see problems in including sources licensed under Apache-2 in this project and would like to do without them.

Long story:

On my work I was among other things busy the last half year to examine Chromium, CEF and CefSharp and the licenses contained therein.
In addition to the many licenses of the components and their different storage structure, a closer look revealed that there are some sources which do not correspond to the described license.
For example:

  • Wrong license in the defined dependency file
  • Modifications of the license without the consent of the license holder by copying the source file
  • Mixed licenses, although defined as single/wrong license in dependency file (MIT was stated, but Facebook’s BSD+Patents License was included)

All these things I would like to avoid for this project.

If we include the Apache component in this project, I think it will have the following effects:

  • This project wouldn't be pure MIT anymore, so it would either have to take over the Apache license or it would have to contain the Apache license in the LICENSE file as well as in Maven (which in my eyes only TooTallNate can decide).
  • This includes the current NOTICE as well as possible new ones.
  • A certain amount of administration is required to ensure that the latest version of the component is always used.

Although it is not difficult for us as an OpenSource project to integrate other OpenSource software, this leads to a considerable additional effort for the users of the library in the administration of the licenses and the resulting compliance with these.

In my opinion, there are therefore the following possible solutions:

  • Using the component and the associated license adjustment, etc.
  • Using a dependency implementing the defined component, which can defined via pom.xml.
  • Write it yourself

The last variant would be my favorite.

@marci4
Copy link
Collaborator Author

marci4 commented Feb 28, 2019

Hey @marci4,
Do you know about java.util.zip.Deflater ? Based on this stackoverflow question, he says that java.util.zip.Deflater can produce rfc 1951 compliant output. Since I don't know java.util.zip.Deflater very much, I wanted to ask you about it. Do you think it can get the deflate compression part done or should I write a deflate implementation around this LZ77 implementation ?

I am pretty sure you can use the Deflater, e.g. https://github.com/TakahikoKawasaki/nv-websocket-client is also using it for their implementation.

Best regards,
Marcel

@haruntuncay
Copy link
Collaborator

haruntuncay commented Mar 7, 2019

Hey @marci4,
Hope you're doing well. When you have the time, could you please check the implementation from this gist , I would like to hear your opinion on it, before submitting a PR.
By the way, I have built an example client and tested manually for both single and fragmented frames and it works for both.

@marci4
Copy link
Collaborator Author

marci4 commented Mar 9, 2019 via email

@haruntuncay
Copy link
Collaborator

@marci4, Thank you for taking the time to review it. I will send a PR soon. Have a fun vacation :)

@marci4 marci4 added this to the Release 1.5.0 milestone May 7, 2019
marci4 added a commit that referenced this issue Mar 17, 2020
Add PerMessageDeflate Extension support, see #574
@marci4
Copy link
Collaborator Author

marci4 commented Mar 17, 2020

Fixed with #866

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development
  
To do
Development

Successfully merging a pull request may close this issue.

3 participants