src/hb-open-type-private.hh | 11 ++ src/hb-ot-layout-common-private.hh | 152 ++++++++++++++++++++++++++++++------- src/hb-ot-layout.cc | 93 +++------------------- 3 files changed, 150 insertions(+), 106 deletions(-)
New commits: commit efe252e6000558f78075adadb2a3dba25ab67c04 Author: Behdad Esfahbod <[email protected]> Date: Mon Dec 17 23:21:05 2012 -0500 [OTLayout] Fix 'size' featureParams implementation Looks at alternate location now. diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh index 50ffa20..bff383a 100644 --- a/src/hb-ot-layout-common-private.hh +++ b/src/hb-ot-layout-common-private.hh @@ -261,7 +261,71 @@ struct FeatureParamsSize { inline bool sanitize (hb_sanitize_context_t *c) { TRACE_SANITIZE (this); - return TRACE_RETURN (c->check_struct (this)); + if (unlikely (!c->check_struct (this))) return TRACE_RETURN (false); + + /* This subtable has some "history", if you will. Some earlier versions of + * Adobe tools calculated the offset of the FeatureParams sutable from the + * beginning of the FeatureList table! Now, that is dealt with in the + * Feature implementation. But we still need to be able to tell junk from + * real data. Note: We don't check that the nameID actually exists. + * + * Read Roberts wrote on 9/15/06 on [email protected] : + * + * Yes, it is correct that a new version of the AFDKO (version 2.0) will be + * coming out soon, and that the makeotf program will build a font with a + * 'size' feature that is correct by the specification. + * + * The specification for this feature tag is in the "OpenType Layout Tag + * Registry". You can see a copy of this at: + * http://partners.adobe.com/public/developer/opentype/index_tag8.html#size + * + * Here is one set of rules to determine if the 'size' feature is built + * correctly, or as by the older versions of MakeOTF. You may be able to do + * better. + * + * Assume that the offset to the size feature is according to specification, + * and make the following value checks. If it fails, assume the the size + * feature is calculated as versions of MakeOTF before the AFDKO 2.0 built it. + * If this fails, reject the 'size' feature. The older makeOTF's calculated the + * offset from the beginning of the FeatureList table, rather than from the + * beginning of the 'size' Feature table. + * + * If "design size" == 0: + * fails check + * + * Else if ("subfamily identifier" == 0 and + * "range start" == 0 and + * "range end" == 0 and + * "range start" == 0 and + * "menu name ID" == 0) + * passes check: this is the format used when there is a design size + * specified, but there is no recommended size range. + * + * Else if ("design size" < "range start" or + * "design size" > "range end" or + * "range end" <= "range start" or + * "menu name ID" < 256 or + * "menu name ID" > 32767 or + * menu name ID is not a name ID which is actually in the name table) + * fails test + * Else + * passes test. + */ + + if (!designSize) + return TRACE_RETURN (false); + else if (subfamilyID == 0 && + subfamilyNameID == 0 && + rangeStart == 0 && + rangeEnd == 0) + return TRACE_RETURN (true); + else if (designSize < rangeStart || + designSize > rangeEnd || + subfamilyNameID < 256 || + subfamilyNameID > 32767) + return TRACE_RETURN (false); + else + return TRACE_RETURN (true); } USHORT designSize; /* Represents the design size in 720/inch @@ -343,9 +407,7 @@ struct FeatureParamsCharacterVariants return TRACE_RETURN (c->check_struct (this) && characters.sanitize (c)); } - /* TODO: This is made private since we don't have the facilities in - * FeatureParams to correctly sanitize this. */ - private: + USHORT format; /* Format number is set to 0. */ USHORT featUILableNameID; /* The ânameâ table name ID that * specifies a string (or strings, @@ -380,24 +442,25 @@ struct FeatureParamsCharacterVariants struct FeatureParams { - /* Note: - * - * FeatureParams structures unfortunately don't have a generic length argument, - * so their length depends on the feature name / requested use. We don't have - * that information at sanitize time. As such, we sanitize for the longest - * subtable possible. This may nuke a possibly valid subtable if it's unfortunate - * enough to happen at the very end of the GSUB/GPOS table. But that's very - * unlikely (I hope!). - * - * When we fully implement FeatureParamsCharacterVariants, we should fix this - * shortcoming... - */ - - inline bool sanitize (hb_sanitize_context_t *c) { + inline bool sanitize (hb_sanitize_context_t *c, hb_tag_t tag) { TRACE_SANITIZE (this); - return TRACE_RETURN (c->check_struct (this)); + if (tag == HB_TAG ('s','i','z','e')) + return TRACE_RETURN (u.size.sanitize (c)); + if ((tag & 0xFFFF0000) == HB_TAG ('s','s','\0','\0')) /* ssXX */ + return TRACE_RETURN (u.stylisticSet.sanitize (c)); + if ((tag & 0xFFFF0000) == HB_TAG ('c','v','\0','\0')) /* cvXX */ + return TRACE_RETURN (u.characterVariants.sanitize (c)); + return TRACE_RETURN (true); } + inline const FeatureParamsSize& get_size_params (hb_tag_t tag) const + { + if (tag == HB_TAG ('s','i','z','e')) + return u.size; + return Null(FeatureParamsSize); + } + + private: union { FeatureParamsSize size; FeatureParamsStylisticSet stylisticSet; @@ -426,26 +489,36 @@ struct Feature if (unlikely (!(c->check_struct (this) && lookupIndex.sanitize (c)))) return TRACE_RETURN (false); + /* Some earlier versions of Adobe tools calculated the offset of the + * FeatureParams subtable from the beginning of the FeatureList table! + * + * If sanitizing "failed" for the FeatureParams subtable, try it with the + * alternative location. We would know sanitize "failed" if old value + * of the offset was non-zero, but it's zeroed now. + */ + Offset orig_offset = featureParams; - if (likely (featureParams.sanitize (c, this))) + if (unlikely (!featureParams.sanitize (c, this, closure ? closure->tag : HB_TAG_NONE))) + return TRACE_RETURN (false); + + if (likely (!orig_offset)) return TRACE_RETURN (true); - /* Some earlier versions of Adobe tools calculated the offset of the - * FeatureParams sutable from the beginning of the FeatureList table! - * Try that instead... */ - if (closure && closure->list_base) + if (featureParams == 0 && closure && closure->list_base && closure->list_base < this) { + unsigned int new_offset_int = (unsigned int) orig_offset - + ((char *) this - (char *) closure->list_base); + Offset new_offset; - new_offset.set (orig_offset - ((char *) this - (char *) closure->list_base)); /* Check that it did not overflow. */ - if (new_offset != (orig_offset - ((char *) this - (char *) closure->list_base))) + new_offset.set (new_offset_int); + if (new_offset == new_offset_int && + featureParams.try_set (c, new_offset) && + !featureParams.sanitize (c, this, closure ? closure->tag : HB_TAG_NONE)) return TRACE_RETURN (false); - - return TRACE_RETURN (featureParams.try_set (c, new_offset) && - featureParams.sanitize (c, this)); } - return TRACE_RETURN (false); + return TRACE_RETURN (true); } OffsetTo<FeatureParams> diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index f7a54bb..e4bac0a 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -647,104 +647,39 @@ hb_ot_layout_get_size_params (hb_face_t *face, unsigned int *range_start, /* OUT. May be NULL */ unsigned int *range_end /* OUT. May be NULL */) { - bool ret = false; const OT::GPOS &gpos = _get_gpos (face); + const hb_tag_t tag = HB_TAG ('s','i','z','e'); unsigned int num_features = gpos.get_feature_count (); for (unsigned int i = 0; i < num_features; i++) { - if (HB_TAG ('s','i','z','e') == gpos.get_feature_tag (i)) + if (tag == gpos.get_feature_tag (i)) { const OT::Feature &f = gpos.get_feature (i); - const OT::FeatureParamsSize ¶ms = f.get_feature_params ().u.size; - - /* This subtable has some "history", if you will. Some earlier versions of - * Adobe tools calculated the offset of the FeatureParams sutable from the - * beginning of the FeatureList table! Now, we don't check for that possibility, - * but we want to at least detect junk data and reject it. - * - * Read Roberts wrote on 9/15/06 on [email protected] : - * - * Yes, it is correct that a new version of the AFDKO (version 2.0) will be - * coming out soon, and that the makeotf program will build a font with a - * 'size' feature that is correct by the specification. - * - * The specification for this feature tag is in the "OpenType Layout Tag - * Registry". You can see a copy of this at: - * http://partners.adobe.com/public/developer/opentype/index_tag8.html#size - * - * Here is one set of rules to determine if the 'size' feature is built - * correctly, or as by the older versions of MakeOTF. You may be able to do - * better. - * - * Assume that the offset to the size feature is according to specification, - * and make the following value checks. If it fails, assume the the size - * feature is calculated as versions of MakeOTF before the AFDKO 2.0 built it. - * If this fails, reject the 'size' feature. The older makeOTF's calculated the - * offset from the beginning of the FeatureList table, rather than from the - * beginning of the 'size' Feature table. - * - * If "design size" == 0: - * fails check - * - * Else if ("subfamily identifier" == 0 and - * "range start" == 0 and - * "range end" == 0 and - * "range start" == 0 and - * "menu name ID" == 0) - * passes check: this is the format used when there is a design size - * specified, but there is no recommended size range. - * - * Else if ("design size" < "range start" or - * "design size" > "range end" or - * "range end" <= "range start" or - * "menu name ID" < 256 or - * "menu name ID" > 32767 or - * menu name ID is not a name ID which is actually in the name table) - * fails test - * Else - * passes test. - */ - - if (!params.designSize) - ret = false; - else if (params.subfamilyID == 0 && - params.subfamilyNameID == 0 && - params.rangeStart == 0 && - params.rangeEnd == 0) - ret = true; - else if (params.designSize < params.rangeStart || - params.designSize > params.rangeEnd || - params.subfamilyNameID < 256 || - params.subfamilyNameID > 32767) - ret = false; - else - ret = true; + const OT::FeatureParamsSize ¶ms = f.get_feature_params ().get_size_params (tag); -#define PARAM(a, A) if (a) *a = params.A - if (ret) + if (params.designSize) { +#define PARAM(a, A) if (a) *a = params.A PARAM (design_size, designSize); PARAM (subfamily_id, subfamilyID); PARAM (subfamily_name_id, subfamilyNameID); PARAM (range_start, rangeStart); PARAM (range_end, rangeEnd); - break; - } #undef PARAM + + return true; + } } } #define PARAM(a, A) if (a) *a = 0 - if (!ret) - { - PARAM (design_size, designSize); - PARAM (subfamily_id, subfamilyID); - PARAM (subfamily_name_id, subfamilyNameID); - PARAM (range_start, rangeStart); - PARAM (range_end, rangeEnd); - } + PARAM (design_size, designSize); + PARAM (subfamily_id, subfamilyID); + PARAM (subfamily_name_id, subfamilyNameID); + PARAM (range_start, rangeStart); + PARAM (range_end, rangeEnd); #undef PARAM - return ret; + return false; } commit e77b4425746ac9eb407ca4e742d962f1955971b4 Author: Behdad Esfahbod <[email protected]> Date: Mon Dec 17 18:42:59 2012 -0500 [OTLayout] Fix tracing diff --git a/src/hb-open-type-private.hh b/src/hb-open-type-private.hh index 6b9f721..5bfeb16 100644 --- a/src/hb-open-type-private.hh +++ b/src/hb-open-type-private.hh @@ -255,7 +255,8 @@ struct hb_sanitize_context_t "may_edit(%u) [%p..%p] (%d bytes) in [%p..%p] -> %s", this->edit_count, p, p + len, len, - this->start, this->end); + this->start, this->end, + this->writable ? "GRANTED" : "DENIED"); return TRACE_RETURN (this->writable); } commit 9b54562d63f1a9e0e5b33d71c32bd1588759ebf1 Author: Behdad Esfahbod <[email protected]> Date: Mon Dec 17 13:55:36 2012 -0500 [OTLayout] Towards correct FeatureParams handling diff --git a/src/hb-open-type-private.hh b/src/hb-open-type-private.hh index 347e299..6b9f721 100644 --- a/src/hb-open-type-private.hh +++ b/src/hb-open-type-private.hh @@ -704,7 +704,13 @@ struct GenericOffsetTo : OffsetType return TRACE_RETURN (likely (obj.sanitize (c, user_data)) || neuter (c)); } - private: + inline bool try_set (hb_sanitize_context_t *c, const OffsetType &v) { + if (c->may_edit (this, this->static_size)) { + this->set (v); + return true; + } + return false; + } /* Set the offset to Null */ inline bool neuter (hb_sanitize_context_t *c) { if (c->may_edit (this, this->static_size)) { diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh index 1671717..50ffa20 100644 --- a/src/hb-ot-layout-common-private.hh +++ b/src/hb-ot-layout-common-private.hh @@ -423,8 +423,29 @@ struct Feature inline bool sanitize (hb_sanitize_context_t *c, const Record<Feature>::sanitize_closure_t *closure) { TRACE_SANITIZE (this); - return TRACE_RETURN (c->check_struct (this) && lookupIndex.sanitize (c) && - featureParams.sanitize (c, this)); + if (unlikely (!(c->check_struct (this) && lookupIndex.sanitize (c)))) + return TRACE_RETURN (false); + + Offset orig_offset = featureParams; + if (likely (featureParams.sanitize (c, this))) + return TRACE_RETURN (true); + + /* Some earlier versions of Adobe tools calculated the offset of the + * FeatureParams sutable from the beginning of the FeatureList table! + * Try that instead... */ + if (closure && closure->list_base) + { + Offset new_offset; + new_offset.set (orig_offset - ((char *) this - (char *) closure->list_base)); + /* Check that it did not overflow. */ + if (new_offset != (orig_offset - ((char *) this - (char *) closure->list_base))) + return TRACE_RETURN (false); + + return TRACE_RETURN (featureParams.try_set (c, new_offset) && + featureParams.sanitize (c, this)); + } + + return TRACE_RETURN (false); } OffsetTo<FeatureParams> commit 87e43b7f2be25840748f920ca33ff553833da45f Author: Behdad Esfahbod <[email protected]> Date: Fri Dec 14 17:48:23 2012 -0500 [OTLayout] Wire tag and list start all the way to Feature To fix FeatureParam issues. No actual fix yet, just plumbing. diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh index da6c8f9..1671717 100644 --- a/src/hb-ot-layout-common-private.hh +++ b/src/hb-ot-layout-common-private.hh @@ -60,9 +60,14 @@ struct Record return tag.cmp (a); } + struct sanitize_closure_t { + hb_tag_t tag; + void *list_base; + }; inline bool sanitize (hb_sanitize_context_t *c, void *base) { TRACE_SANITIZE (this); - return TRACE_RETURN (c->check_struct (this) && offset.sanitize (c, base)); + const sanitize_closure_t closure = {tag, base}; + return TRACE_RETURN (c->check_struct (this) && offset.sanitize (c, base, &closure)); } Tag tag; /* 4-byte Tag identifier */ @@ -192,7 +197,8 @@ struct LangSys return reqFeatureIndex;; } - inline bool sanitize (hb_sanitize_context_t *c) { + inline bool sanitize (hb_sanitize_context_t *c, + const Record<LangSys>::sanitize_closure_t * = NULL) { TRACE_SANITIZE (this); return TRACE_RETURN (c->check_struct (this) && featureIndex.sanitize (c)); } @@ -230,7 +236,8 @@ struct Script inline bool has_default_lang_sys (void) const { return defaultLangSys != 0; } inline const LangSys& get_default_lang_sys (void) const { return this+defaultLangSys; } - inline bool sanitize (hb_sanitize_context_t *c) { + inline bool sanitize (hb_sanitize_context_t *c, + const Record<Script>::sanitize_closure_t * = NULL) { TRACE_SANITIZE (this); return TRACE_RETURN (defaultLangSys.sanitize (c, this) && langSys.sanitize (c, this)); } @@ -413,7 +420,8 @@ struct Feature inline const FeatureParams &get_feature_params (void) const { return this+featureParams; } - inline bool sanitize (hb_sanitize_context_t *c) { + inline bool sanitize (hb_sanitize_context_t *c, + const Record<Feature>::sanitize_closure_t *closure) { TRACE_SANITIZE (this); return TRACE_RETURN (c->check_struct (this) && lookupIndex.sanitize (c) && featureParams.sanitize (c, this));
_______________________________________________ HarfBuzz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/harfbuzz
