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

EndOfStream bug in LZWInputStream #63

Closed
mr-miles opened this issue Dec 17, 2014 · 17 comments
Closed

EndOfStream bug in LZWInputStream #63

mr-miles opened this issue Dec 17, 2014 · 17 comments
Assignees
Labels
bug documentation lzw ready PR ready for merging when appropriate
Milestone

Comments

@mr-miles
Copy link

Hi,

At the end of the stream, LZWInputStream returns -1 as the bytes read, which makes any stream reader throw an exception. The ReadToEnd() call on the streamreader always throws as per this test (with any LZW compressed file):

        using (var file = new FileStream(testFile, FileMode.Open))
        using (var lzw = new LzwInputStream(file))
        using (var reader = new StreamReader(lzw))
        {
            reader.ReadToEnd();
        }

The same test using this class can successfully read the stream:

public class FixedLzwInputStream : LzwInputStream
{
    public FixedLzwInputStream(Stream baseInputStream)
        : base(baseInputStream)
    { }

    public override int Read(byte[] buffer, int offset, int count)
    {
        var res = base.Read(buffer, offset, count);
        return res == -1 ? 0 : res;
    }
}

It took me ages to work out what was going on ... and I found there is a pull request to fix this:
#10

Please could it be merged? I can contribute a test for this if that is currently a blocker

@jfreilly
Copy link
Member

jfreilly commented Jan 8, 2015

This is a little tricky in that the code was dumped in by me but the licensing for the source code was never sorted out. It was never included in the project and never tested. It was in the todo list which never got done The fix itself is trivial so could be done but the licensing issue may be harder to sort out.

@mr-miles
Copy link
Author

mr-miles commented Jan 9, 2015

I see: you do know it's included in the nuget package?

On Thursday, 8 January 2015, John Reilly notifications@github.com wrote:

This is a little tricky in that the code was dumped in by me but the
licensing for the source code was never sorted out. It was never included
in the project and never tested. It was in the todo list which never got
done The fix itself is trivial so could be done but the licensing issue may
be harder to sort out.


Reply to this email directly or view it on GitHub
#63 (comment)
.

@jfreilly
Copy link
Member

No I had nothing to so with the nuget package

On Fri, Jan 9, 2015 at 9:37 PM, mr-miles notifications@github.com wrote:

I see: you do know it's included in the nuget package?

On Thursday, 8 January 2015, John Reilly notifications@github.com
wrote:

This is a little tricky in that the code was dumped in by me but the
licensing for the source code was never sorted out. It was never
included
in the project and never tested. It was in the todo list which never got
done The fix itself is trivial so could be done but the licensing issue
may
be harder to sort out.


Reply to this email directly or view it on GitHub
<
https://github.com/icsharpcode/SharpZipLib/issues/63#issuecomment-69271859>

.


Reply to this email directly or view it on GitHub
#63 (comment)
.

@McNeight
Copy link
Contributor

McNeight commented Apr 14, 2016

This bug was fixed with c95d302 and can be closed.

@christophwille

I'm not sure what licensing issue @jfreilly is referring to, but it might be about the expired LZW patent?

-Neil

@christophwille
Copy link
Member

To me @jfreilly (please chime in) the license comment seems like the code file's origins were never of the proper license (and the file was intended for reference initially).

@jfreilly
Copy link
Member

The code originates from an email whose origins are unknown.

@McNeight
Copy link
Contributor

Then, going forward, should we just delete this code completely?

@McNeight McNeight added the lzw label Apr 16, 2016
@jfreilly
Copy link
Member

Yes it's the only real option I think.

Cheers john.

On Sat, 16 Apr 2016 15:00 Neil McNeight notifications@github.com wrote:

Then, going forward, should we just delete this code completely?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#63 (comment)

@McNeight
Copy link
Contributor

Perhaps not. After a bit of digging through Google, it would appear I've found the source of the source.

Now, if the magic incantation of @gburca works correctly, then perhaps we may even get this bit of code relicensed to MIT as per issue #103.

-Neil

@jfreilly
Copy link
Member

Ok sounds cool, nice digging.

@gburca
Copy link

gburca commented Apr 19, 2016

I'm fine with releasing it under MIT. What's the best way of doing that? Should I modify the license and send a pull request?

@gburca
Copy link

gburca commented Apr 19, 2016

Looks like @McNeight modified the copyright in 3fae4cf. While I'm OK with licensing the code under MIT, I'm NOT OK with relinquishing the copyright.

@McNeight
Copy link
Contributor

@gburca

No one has relinquished copyright. The main contributors, myself included, have signed a Joint Copyright Assignment, under which each contributor continues to own the copyright on their contribution.

@christophwille has been leading the effort to relicense to MIT. We have an open issue for obtaining the proper rights in order to relicense here. Up until now, the only thing he has required from patch contributors (>5 lines) is the following statement:

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.

With all that said, your contribution is fairly sizeable. I don't know where the line is between needing a signed JCA and simply adding the above statement. Before I do anything, I'd prefer that Chris respond first.

-Neil

@christophwille
Copy link
Member

Even in SharpDevelop, PRs only require that paragraph. Contributors with full commit permission to the repository are asked for the JCA, simply so we have full documentation on their work (otherwise it would be kind of hard to track).

@McNeight
Copy link
Contributor

If I am understanding it correctly then, I think what we would like is for @gburca to fork, make a slight modification to the three LZW files, and the submit a PR with the statement in the comment. If we wanted to be pedantic about it, we could delete the files in our repository, and then have him submit a PR to include them again.

Either way, the repository history will remember and track his contribution, he will have verified his claim to the code, maintaining his copyright on it, and we will be able to include it under the MIT license.

Simple enough, eh?

@McNeight
Copy link
Contributor

@gburca

Please let us know if you would like to switch your code to the MIT license and contribute to the project. If we don't receive a response from you soon, your code will be deleted from the repository and this issue will be closed out.

-Neil

@McNeight McNeight added the ready PR ready for merging when appropriate label May 16, 2016
@McNeight
Copy link
Contributor

With the change in license to MIT, as well as the new copyright statement ("Copyright © 2000-2016 SharpZipLib Contributors"), I am going to consider the statement above by @gburca that "While I'm OK with licensing the code under MIT, I'm NOT OK with relinquishing the copyright." as permission to include the LZW code as part of the SharpZipLib distribution.

Gabriel,

Thanks for taking the time to respond, and thank you for your contribution.

-Neil

@McNeight McNeight added this to the 1.0 milestone May 17, 2016
@McNeight McNeight modified the milestone: 1.0 May 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug documentation lzw ready PR ready for merging when appropriate
Projects
None yet
Development

No branches or pull requests

5 participants