getsentry / rust-sourcemap

A library for rust that implements basic sourcemap handling
Other
224 stars 27 forks source link

fix: Prevent infinite loop #90

Closed kdy1 closed 4 months ago

kdy1 commented 4 months ago

Context: https://github.com/swc-project/swc/pull/9050

I used adjust_mapping, but I found sourcemap crate is panicking with integer overflow. The cause was

            while token.get_dst_line() != prev_dst_line {
                rv.push(';');
                prev_dst_line += 1;
            }

so I modified it to

            while token.get_dst_line() > prev_dst_line {
                rv.push(';');
                prev_dst_line += 1;
            }

and it worked.

loewenheim commented 4 months ago

Hi, can you post an example sourcemap for which this change makes a difference? Intuitively it shouldn't, now I'm wondering if there's something wrong with our logic.

loewenheim commented 4 months ago

@Swatinem Yeah, I assumed it was something like this. Out-of-order tokens have never made sense and just shouldn't exist.

kdy1 commented 4 months ago

I did orig_js_map.adjust_mappings(transform_js_map). Both of them are valid so I could print them, but the resulting sourcemap panics on .to_writer.

Source map files are uploaded at https://gist.github.com/kdy1/000301fc6091b3e8d5aec7cef8e89b80

Exact code:

        let mut map = builder.into_sourcemap();

        let mut map_s = vec![];
        map.to_writer(&mut map_s).unwrap();
        std::fs::write("transform.js.map", map_s).unwrap();

        if let Some(orig) = orig {
            let mut map_s = vec![];
            orig.to_writer(&mut map_s).unwrap();
            std::fs::write("orig.js.map", map_s).unwrap();

            let mut copied = orig.clone();
            copied.adjust_mappings(&map);
            map = copied;
        }

        map.to_writer(&mut vec![]).unwrap(); // Fails with a panic caused by an overflow of integer

        map
loewenheim commented 4 months ago

@kdy1 Right. I can confirm that #91 would fix your issue. As @Swatinem says, your PR would fix the infinite loop, but likely not solve the problem—the sourcemap you get out is probably going to be nonsense.

kdy1 commented 4 months ago

Thank you! I agree that my fix is wrong. I tried reproducing it with a single source map but it seems like my fix simply removes erroneous tokens from the output