Closed Bug 527558 Opened 13 years ago Closed 7 years ago
Crash [@ xul
.dll!ns Dependent CSubstring::ns Dependent CSubstring]
Stack overflow in an xsl transform using the key function.
This bug is hit very quickly, so I had to disable key() testing in the fuzzer. No rush, but I can't test key() anymore until this is fixed.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
This seems to catch the recursion. Still need to add a crashtest.
Branch would get this patch without the change to xslt.properties and we'll just report an unknown error there.
Aren't use-expressions allowed to reference keys? As long as that doesn't happen in a circular fashion. I wonder if we should simply do what the js-engine does and have a catch-all recursion detector based on measuring the amount of C++ stack used.
(In reply to comment #6) > Aren't use-expressions allowed to reference keys? As long as that doesn't > happen in a circular fashion. Nope: http://www.w3.org/1999/11/REC-xslt-19991116-errata/#E13.
Attachment #412476 - Flags: review?(jonas) → review+
Comment on attachment 412476 [details] [diff] [review] v2 Why the ignoreError stuff? Couldn't technically XSLT2 allow keys as long as they aren't used circularly? We still create an ErrorExpr, so no risk of out of control recursions. Nit: I would prefer if 'allowed' was called something more descriptive? Like exprTypeAllowed or some such? r=me either way.
(In reply to comment #8) > (From update of attachment 412476 [details] [diff] [review]) > Why the ignoreError stuff? Couldn't technically XSLT2 allow keys as long as > they aren't used circularly? I really don't see how that would fit into 1 of the 6 bullet points in http://www.w3.org/TR/xslt#forwards. And for patterns we don't create ErrorExprs, so use and match would be treated differently? > Nit: I would prefer if 'allowed' was called something more descriptive? Like > exprTypeAllowed or some such? Yeah, I forgot to tell you I'm looking for better names and suggestions are welcome ;-).
Crash Signature: [@ xul.dll!nsDependentCSubstring::nsDependentCSubstring]
Peter: Does this still apply? Should we try to land it?
(In reply to Jonas Sicking (:sicking) from comment #10) > Peter: Does this still apply? Should we try to land it? Needed some updating, thanks for reminding me.
You need to log in before you can comment on or make changes to this bug.