Cap total in-scope namespace bindings - #987
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #987 +/- ##
==========================================
- Coverage 57.31% 55.88% -1.44%
==========================================
Files 46 47 +1
Lines 18197 18337 +140
==========================================
- Hits 10429 10247 -182
- Misses 7768 8090 +322
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
d26e488 to
968737d
Compare
|
Replaces #981 |
| /// almost always on the root element; 128 is significantly more than what most legitimate documents | ||
| /// would declare, while bounding both the heap allocated and the cost of prefix resolution | ||
| /// (which scans the binding stack). | ||
| pub const DEFAULT_MAX_NAMESPACE_BINDINGS: usize = 128; |
There was a problem hiding this comment.
Note: default reduced from 256 to 128. No particular reason for this other than that it felt wildly excessive, and this is still fairly high. If there's a compelling argument otherwise though, I'll change it back.
Replace the per-element `max_declarations_per_element` limit with a total in-scope `max_namespace_bindings` limit (default 128). Documents in excess of this limit are *likely* maliciously constructed, and bounding the stack scanned by `resolve_prefix` prevents this vector from being used to cause DDoS via CPU exhaustion. The check moves from `push()` (which counted declarations per element) into `add()` (which checks `bindings.len()` against the total limit), thereby catching both single-element and cross-element accumulation. Renames: TooManyDeclarations -> TooManyBindings, max_declarations_per_element -> max_namespace_bindings, DEFAULT_MAX_DECLARATIONS_PER_ELEMENT -> DEFAULT_MAX_NAMESPACE_BINDINGS. closes tafia#980
| } else if namespace == RESERVED_NAMESPACE_XMLNS.1 { | ||
| // error, non-`xmlns` prefix set to xmlns uri |
There was a problem hiding this comment.
Originally, I placed the comment on the line before if to keep the nice formatting of else if statement. Don't know why rustfmt does not have the ability to keep else and if on different lines to keep the indentation of conditions the same.
3 participants
Replace the per-element
max_declarations_per_elementlimit with a total in-scopemax_namespace_bindingslimit (default 128). Documents in excess of this limit are likely maliciously constructed, and bounding the stack scanned byresolve_prefixprevents this vector from being used to cause DDoS via CPU exhaustion.The check moves from
push()(which counted declarations per element) intoadd()(which checksbindings.len()against the total limit), thereby catching both single-element and cross-element accumulation.Renames:
TooManyDeclarations -> TooManyBindings,
max_declarations_per_element -> max_namespace_bindings,
DEFAULT_MAX_DECLARATIONS_PER_ELEMENT -> DEFAULT_MAX_NAMESPACE_BINDINGS.
closes #980