-
Notifications
You must be signed in to change notification settings - Fork 176
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
Investigate removing scalar value validation from HarfBuzz canonical composition and decomposition #2840
Comments
For decomposition, it should be fine to make the ICU4X code operate on For composition, ICU4X internals seem sufficiently reliant on having assurances of scalar valueness that either we should validate the inputs provided by HarfBuzz or take the position that if HarfBuzz passes non-scalar values that's the kind of error in using the HarfBuzz API that UB is acceptable. |
The HarfBuzz API for bypassing input validation now says the caller is responsible for passing in scalar values into HarfBuzz: harfbuzz/harfbuzz@f2297e6 We could now take the position that calling |
I don't think it's worth over-engineering this to avoid a validation which is essentially free. CC @echeran |
That sounds reasonable. As I see it, either way, it's not an error that is actionable by the user, so UB without crashing seems acceptable indeed. FWIW, I think this is also in line with the proposed rules of thumb in the data safety PR.
That makes sense. And at least we're passing Rust |
Ping @hsivonen |
Sorry about missing that this was blocking on me. My preference is that we trust that scalar values that we receive from HarfBuzz are indeed scalar values and, therefore, can be interpreted as Rust |
Ok, seems reasonable. In any case I'd prefer to fix this in the harfbuzz glue code in the upstream repo so that we avoid interacting with raw harfbuzz types in the icu repo. #3257 |
The initial glue code for providing the canonical composition and canonical decomposition operation to HarfBuzz assumes that the incoming
hb_codepoint_t
values might not be valid scalar values.We should investigate avoiding the validation overhead by:
u32
-argument alternatives to thechar
-argument methods.The text was updated successfully, but these errors were encountered: