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

Internal compression is required and faulty directories cause panic #202

Open
tdewolff opened this issue Jan 10, 2025 · 3 comments
Open

Comments

@tdewolff
Copy link

The internal compression seems to be required to be Gzip, but the spec says it's optional?

Besides, this package could use some proper error checking. For faulty directories that point to something that is not Gzip encoded, the Gzip decoder isn't checked for errors and later on causes a nil reference panic. The verify functionality would be very useful if it'd do a lot more checking with helpful messages.

@bdon
Copy link
Member

bdon commented Jan 10, 2025

The spec is future-proof relative to this implementation. It hasn't been a priority to implement Brotli or Zstd compression yet, because many use cases of PMTiles are intended to be decoded in the browser, and Gzip has a polyfill for the missing DecompressionStream browser feature.

We may be at the point where the vast majority of browsers have DecompressionStream('br') compared to a few years ago, so it's worth doing some benchmarking to see what the gain is. Or we could just hold out until DecompressionStream('zstd') is everywhere.

CanIUse: https://caniuse.com/?search=DecompressionStream

@tdewolff
Copy link
Author

That's OK, but it doesn't support no compression either, and doesn't check. It just blindly decodes with Gzip and doesn't even check the error. In general, I would expect the verify functionality to do more checking to be useful.

@bdon
Copy link
Member

bdon commented Jan 10, 2025

Sorry, you're right, that's an oversight on my part - the go program doesn't support the no-compression option right now. It shouldn't be too difficult to add.

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

No branches or pull requests

2 participants