@Swatinem I tried to find a crate that does this before implementing, but I could not find one. I agree, using a crate would be the ideal solution if one exists

The code doesn't look like it even has to guarantee UTF-8 Validity, isn't it fine if the file contents are just copied as bytes? A reader must always ensure correctness anyways.

The code doesn't look like it even has to guarantee UTF-8 Validity, isn't it fine if the file contents are just copied as bytes? A reader must always ensure correctness anyways.

@Dav1dde, @loewenheim added this in #816. The context is not entirely clear to me from that PR, but if I remember correctly, I think the reason it needed to be added was to have client-side verification that users upload valid source bundles to Sentry. Valid source bundles can only contain UTF-8 encoded data.

I think I have addressed all feedback. Please let me know if I missed something

I have verified with the memory profiler that when using a version of symbolic with this change, sentry-cli's total memory usage during a sourcemap upload is reduced, since we no longer allocate memory for the entire file.

Hey @loewenheim, just saw this:

Have you verified that if the UTF8Reader errors, nothing is written to the writer by io::copy? Also, this type would be ripe for fuzzing/property-based testing (generat e random strings and see if they get read correctly).

Regarding the first question: no, I have not tested this, but I would expect that we would write everything to io::copy up until the error occurs. UTF8Reader performs the validation lazily as it reads from the stream it is wrapping, so whether we write anything to the writer depends on where the first UTF8 violation occurs in the reader stream, and how much io::copy reads at a time. I think the only way to guarantee we don't write anything to the writer in an error scenario (without loading the entire reader into memory) would be to io::copy the stream into a temp file, then copy the temp file to the writer stream. Do you think we need to do this?

As for the fuzzing/property based testing, I am not sure how to do this, but it sounds like a good idea!

My worry is this: if we write into the writer up to the first read error, how does that interact with add_file_skip_read_failed? Won't that mess up the sourcebundle?

As for proptesting, I've used it a fair bit and would be happy to give you an introduction

@loewenheim, I have addressed your feedback