Skip to content

Decompressed Stream Support #10

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

Merged
merged 18 commits into from
May 5, 2019
Merged

Conversation

fairingrey
Copy link
Contributor

@fairingrey fairingrey commented May 2, 2019

Description

This PR seeks to add support for decompressing streams of encoded bytes under each of the encoding algorithms used.

This is the current progress:

  • Brotli
  • Deflate
  • Gzip
  • Zlib

Motivation and Context

Coming up with a standard pattern of decompressing encoded streams that the library already provides would be valuable for various use cases.

It would also unblock progress on http-rs/tide#194.

How Has This Been Tested?

Tests are added.

  • Brotli
  • Deflate
  • Gzip
  • Zlib

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@fairingrey fairingrey force-pushed the decompression branch 3 times, most recently from 21b63e1 to 7faf317 Compare May 2, 2019 18:54
@fairingrey fairingrey marked this pull request as ready for review May 2, 2019 18:55
@fairingrey
Copy link
Contributor Author

r? @Nemo157

and a bit of refactoring adding the bytes
@Nemo157 Nemo157 self-assigned this May 2, 2019
@Nemo157
Copy link
Member

Nemo157 commented May 3, 2019

Overall looks good, just a couple of small issues in the flate compression.

@fairingrey fairingrey mentioned this pull request May 3, 2019
8 tasks
@fairingrey
Copy link
Contributor Author

fairingrey commented May 4, 2019

@Nemo157 I think I'm ready for another review.

EDIT: Well, I could write more tests, actually (and probably ones that don't depend on implementations native to this crate), but I'll ask more about them later.

fairingrey added 3 commits May 4, 2019 17:23
I believe I know how to refactor this, but might need a bit help
@fairingrey
Copy link
Contributor Author

fairingrey commented May 5, 2019

A remark on using Bytes::extend_from_slice here:

https://github.com/rustasync/async-compression-rs/blob/014a2e101d9b044517399d9006872d84efb6c621/src/stream/gzip.rs#L255

The only time(s) this is reached is when the input is too small be read as data, as the footer will always take up 8 bytes, and the check here:

https://github.com/rustasync/async-compression-rs/blob/014a2e101d9b044517399d9006872d84efb6c621/src/stream/gzip.rs#L265

Prevents the footer from being read as data. So I think this is the best I can get it without it freaking out on any odd/malformed data.

Let's go with one more review, and I think this should be good? @Nemo157

@fairingrey
Copy link
Contributor Author

Oh right, I forgot the inner stream can be pending. I'll fix it up right now.

@Nemo157 Nemo157 merged commit 47bdfe2 into Nullus157:master May 5, 2019
@Nemo157
Copy link
Member

Nemo157 commented May 5, 2019

Awesome, now I'll rebase the proptest branch on this and see if I can break it 😈

@fairingrey fairingrey deleted the decompression branch May 5, 2019 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants