Fix reading compressed WOFF data

[brianpopow]

Prerequisites

• [x] I have written a descriptive pull-request title
• [x] I have verified that there are no overlapping pull-requests open
• [x] I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules :cop:.
• [ ] I have provided test coverage for my change (where applicable)

Description

This PR fixes reading compressed woff data: Read data in a loop. The first read does not ensure all data is read.

Fix issue #296

Note: I cant use the font file provided in #296 for a unit test. Not sure howto make a test without a sample file.

[codecov[bot]]

Codecov Report

Merging #297 (22b292f) into main (323ea06) will increase coverage by 0%. The diff coverage is 75%.

@@          Coverage Diff          @@
##            main    #297   +/-   ##
=====================================
  Coverage     83%     83%           
=====================================
  Files        222     222           
  Lines      12271   12276    +5     
  Branches    1780    1781    +1     
=====================================
+ Hits       10248   10253    +5     
  Misses      1591    1591           
  Partials     432     432           

Flag	Coverage Δ
unittests	83% <75%> (+<1%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/SixLabors.Fonts/Tables/Woff/WoffTableHeader.cs	92% <75%> (+2%)	:arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[dlemstra]

This probably deserved a unit test? With a custom stream that reads the bytes in blocks?

[brianpopow]

This probably deserved a unit test? With a custom stream that reads the bytes in blocks?

@dlemstra I agree that we should have tests for this, but I dont think a custom stream would be enough as this issue is specifically related to a change in how .Net 6.0 handles ZlibInflateStream.Read.

We currently do not run the test for .Net6.0 and I think we should. In fact the issue here is already covered by tests, we just need to target .Net 6.0 with the tests (I have tried that).

@JimBobSquarePants Is there a reason we are still targeting the old frameworks? Like netcoreapp3.1;netcoreapp2.1;net472. Should we drop those and replace them with .Net6.0?

[JimBobSquarePants]

I would absolutely love to drop those targets but that means dropping them for ImageSharp.Drawing also.

What do we think @SixLabors/core ?

[brianpopow]

I would absolutely love to drop those targets but that means dropping them for ImageSharp.Drawing also.

What do we think @SixLabors/core ?

My opinion is, we should drop support for the old frameworks. I dont see why we should support them, when even microsoft does not do that.

[mj2015]

Can I ask how often releases are done? If I understand correctly, there is a fix in the code for this issue, but there hasn't been a release done since the beginning of July. I'd really like access to the fix.

As for maintaining backward compatibility, Core 3.1 is going out of support, and I'm only using .Net6 in any new work. Given the library is pretty much for newer projects, I can't see why you'd keep support. Thanks.

[JimBobSquarePants]

We're trying to limit Nuget beta releases while we work on an RC release, however, every build against our main branch is published on our MyGet feed.

[mj2015]

Thank you for that - I can confirm that the fix works well here.

Also a further thought on the dropping of old frameworks - it is based on my thinking that this project is primarily aimed at developers who are wanting multi-platform operation, and that is .Net 6 right now. If you are on framework, you'll use the Windows specifics, and if you are on an old Core version, it isn't too hard to update. Thanks again.