search

harfbuzz / rustybuzz

A complete harfbuzz's shaping algorithm port to Rust
MIT License
551 stars 37 forks source link

Panic when shaping with fonts "Malayalam MN" and "Malayalam Sangam MN" #131

Closed maxmelander closed 2 months ago

maxmelander commented 2 months ago

When trying to shape with the MacOS fonts "Malayalam MN" or "Malayalam Sangam MN", the ttf-parser panics with: panicked at tf-parser-cef4d149453e6ac0/85c1ff2/src/parser.rs:767:44:attempt to add with overflow

The stack trace is something like:

> at engine.wasm.ttf_parser::parser::Stream::read_bytes::hfe0c36c236a2369c

> at engine.wasm.rustybuzz::hb::aat_layout_kerx_table::apply::hcd330d6cacc8e120

> at engine.wasm.rustybuzz::hb::ot_shape::shape_internal::h7cd2d2eece5d86ff

> at engine.wasm.rustybuzz::hb::shape::shape_with_plan::h18a9f9cbff5f2f6eJ

This is tested with our own fork of cosmic-text, using the latest commit of rustybuzz, running in a wasm context. The font is loaded with the Local Font Access API

LaurenzV commented 2 months ago

What text are you shaping?

LaurenzV commented 2 months ago

I tried this for example and can't really reproduce it: cargo run --example shape /System/Library/Fonts/Supplemental/Malayalam\ MN.ttc "മലയാളം"

maxmelander commented 2 months ago

In this example, "abc". But seems to fail with any string. Just tested with "ഹലോ" as well with the same result.

maxmelander commented 2 months ago

Okay, thanks for trying it out. I will try to investigate a bit further and see if I can pinpoint where the problem is coming from.

LaurenzV commented 2 months ago

Yeah, that would help. 👍 I can't reproduce it at all, unfortunately.

RazrFalcon commented 2 months ago

@LaurenzV Perhaps this is because WASM is 32bit?

RazrFalcon commented 2 months ago

Looks like one of the lens passed to ttf_parser::Stream::read_bytes is 4294967283. While overflowing isn't great (shouldn't panic in release mode?), I bet we're reading something wrong in the first place.

RazrFalcon commented 2 months ago

Looks like one of kerx subtable's length is set to 4294967283, aka 0xFFFFFFF3, which is very suspicious.

CryZe commented 2 months ago

Maybe it's meant to be a signed integer?

RazrFalcon commented 2 months ago

What a dumb mistake... An iterator was going forever. No idea how it wasn't affecting current tests. Maybe all kerx tables we had so far had just one table, which kinda solves the issue, afaik.