You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Felix Meschberger (JIRA)" <ji...@apache.org> on 2008/08/05 12:08:44 UTC
[jira] Created: (SLING-601) Request Inclusion Counter operating too
rigid
Request Inclusion Counter operating too rigid
---------------------------------------------
Key: SLING-601
URL: https://issues.apache.org/jira/browse/SLING-601
Project: Sling
Issue Type: Bug
Components: Engine
Affects Versions: Engine 2.0.2
Reporter: Felix Meschberger
Assignee: Felix Meschberger
Fix For: Engine 2.0.4
The SlingMainServlet implements a recursion level counter to prevent recursions going too deep in the checkRecursionLevel method. What this method does is incrementing a counter stored as a request attribute everytime RequestDispatcher.include is called (by whatever means).
The drawback of this mechanism is, that the counter is only incremented and never decremented after an inclusion returns. This in fact is not a recursion level counter but a counter limiting the number of times RequestDispatcher.include may be called. Another drawback of this is, that the implementation is overcomplicated, because the RequestData object created for each request keeps a stack of a inclusions and hence could easily implement a real recursion level checker by just checking the size of the stack.
I think, we should drop the SlingMainServlet.checkRecursionLevel method altogether and replace it with a new RequestData.checkAbort method which throws in case any of two cases occurrs:
* Recursion Level is too deep (50 seems like useful)
* Number of includes exceeds a given number (something like 1000 or more ?)
In addition the Exception should be made public in the Sling engine module such that users of any include mechanism could handle it if need be.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-601) Request Inclusion Counter operating
too rigid
Posted by "Tobias Bocanegra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625487#action_12625487 ]
Tobias Bocanegra commented on SLING-601:
----------------------------------------
initially the counter just counted the number of includes. so if you had more than 50 includes on your page it aborted.
imo (and of carstens) this was wrong and he changed it to count nested includes (so decrementing after the include).
endless loops are not easy detectable. there is nothing you can do to prevent people writing a "while(true);". of course there can be counter measures, like killing a 100% thread, or checking that the output of a HTML doesn't get too big...but i think this is a tedious work.
> Request Inclusion Counter operating too rigid
> ---------------------------------------------
>
> Key: SLING-601
> URL: https://issues.apache.org/jira/browse/SLING-601
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Engine 2.0.2
> Reporter: Felix Meschberger
> Assignee: Felix Meschberger
> Fix For: Engine 2.0.4
>
>
> The SlingMainServlet implements a recursion level counter to prevent recursions going too deep in the checkRecursionLevel method. What this method does is incrementing a counter stored as a request attribute everytime RequestDispatcher.include is called (by whatever means).
> The drawback of this mechanism is, that the counter is only incremented and never decremented after an inclusion returns. This in fact is not a recursion level counter but a counter limiting the number of times RequestDispatcher.include may be called. Another drawback of this is, that the implementation is overcomplicated, because the RequestData object created for each request keeps a stack of a inclusions and hence could easily implement a real recursion level checker by just checking the size of the stack.
> I think, we should drop the SlingMainServlet.checkRecursionLevel method altogether and replace it with a new RequestData.checkAbort method which throws in case any of two cases occurrs:
> * Recursion Level is too deep (50 seems like useful)
> * Number of includes exceeds a given number (something like 1000 or more ?)
> In addition the Exception should be made public in the Sling engine module such that users of any include mechanism could handle it if need be.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-601) Request Inclusion Counter operating
too rigid
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12668017#action_12668017 ]
Felix Meschberger commented on SLING-601:
-----------------------------------------
Implemented this change in Rev. 738470:
* RecursionTooDeepException thrown when recursion level is reached
* TooManyCallsException thrown when number of calls is too high
Recursion level and call numbers are configurable for the SlingMainServlet and default to 50 for the recursion level and 1000 for the call counter.
> Request Inclusion Counter operating too rigid
> ---------------------------------------------
>
> Key: SLING-601
> URL: https://issues.apache.org/jira/browse/SLING-601
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Engine 2.0.2
> Reporter: Felix Meschberger
> Assignee: Felix Meschberger
> Fix For: Engine 2.0.4
>
>
> The SlingMainServlet implements a recursion level counter to prevent recursions going too deep in the checkRecursionLevel method. What this method does is incrementing a counter stored as a request attribute everytime RequestDispatcher.include is called (by whatever means).
> The drawback of this mechanism is, that the counter is only incremented and never decremented after an inclusion returns. This in fact is not a recursion level counter but a counter limiting the number of times RequestDispatcher.include may be called. Another drawback of this is, that the implementation is overcomplicated, because the RequestData object created for each request keeps a stack of a inclusions and hence could easily implement a real recursion level checker by just checking the size of the stack.
> I think, we should drop the SlingMainServlet.checkRecursionLevel method altogether and replace it with a new RequestData.checkAbort method which throws in case any of two cases occurrs:
> * Recursion Level is too deep (50 seems like useful)
> * Number of includes exceeds a given number (something like 1000 or more ?)
> In addition the Exception should be made public in the Sling engine module such that users of any include mechanism could handle it if need be.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-601) Request Inclusion Counter operating
too rigid
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12668058#action_12668058 ]
Felix Meschberger commented on SLING-601:
-----------------------------------------
Fixed integration tests to check for the new RecursionTooDeepException in Rev. 738520.
> Request Inclusion Counter operating too rigid
> ---------------------------------------------
>
> Key: SLING-601
> URL: https://issues.apache.org/jira/browse/SLING-601
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Engine 2.0.2
> Reporter: Felix Meschberger
> Assignee: Felix Meschberger
> Fix For: Engine 2.0.4
>
>
> The SlingMainServlet implements a recursion level counter to prevent recursions going too deep in the checkRecursionLevel method. What this method does is incrementing a counter stored as a request attribute everytime RequestDispatcher.include is called (by whatever means).
> The drawback of this mechanism is, that the counter is only incremented and never decremented after an inclusion returns. This in fact is not a recursion level counter but a counter limiting the number of times RequestDispatcher.include may be called. Another drawback of this is, that the implementation is overcomplicated, because the RequestData object created for each request keeps a stack of a inclusions and hence could easily implement a real recursion level checker by just checking the size of the stack.
> I think, we should drop the SlingMainServlet.checkRecursionLevel method altogether and replace it with a new RequestData.checkAbort method which throws in case any of two cases occurrs:
> * Recursion Level is too deep (50 seems like useful)
> * Number of includes exceeds a given number (something like 1000 or more ?)
> In addition the Exception should be made public in the Sling engine module such that users of any include mechanism could handle it if need be.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (SLING-601) Request Inclusion Counter operating too
rigid
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Felix Meschberger closed SLING-601.
-----------------------------------
Resolution: Fixed
There was a small bug in the callCounter handling the RequestData.service method, which caused the max Call limit to never be reached since, the counter was decremented after the call. This is wrong and the counter is now not decremented any more. This also ensures the counter value in the timer name has a sensible and unique value.
Fixed in Rev. 738529.
In Rev. 738531 I added an integration test for the max call limit check, which in fact revealed the issue the RequestData.service method.
> Request Inclusion Counter operating too rigid
> ---------------------------------------------
>
> Key: SLING-601
> URL: https://issues.apache.org/jira/browse/SLING-601
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Engine 2.0.2
> Reporter: Felix Meschberger
> Assignee: Felix Meschberger
> Fix For: Engine 2.0.4
>
>
> The SlingMainServlet implements a recursion level counter to prevent recursions going too deep in the checkRecursionLevel method. What this method does is incrementing a counter stored as a request attribute everytime RequestDispatcher.include is called (by whatever means).
> The drawback of this mechanism is, that the counter is only incremented and never decremented after an inclusion returns. This in fact is not a recursion level counter but a counter limiting the number of times RequestDispatcher.include may be called. Another drawback of this is, that the implementation is overcomplicated, because the RequestData object created for each request keeps a stack of a inclusions and hence could easily implement a real recursion level checker by just checking the size of the stack.
> I think, we should drop the SlingMainServlet.checkRecursionLevel method altogether and replace it with a new RequestData.checkAbort method which throws in case any of two cases occurrs:
> * Recursion Level is too deep (50 seems like useful)
> * Number of includes exceeds a given number (something like 1000 or more ?)
> In addition the Exception should be made public in the Sling engine module such that users of any include mechanism could handle it if need be.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-601) Request Inclusion Counter operating
too rigid
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625475#action_12625475 ]
Felix Meschberger commented on SLING-601:
-----------------------------------------
I think the counter should prevent to deeply nested recursions, so the first situation should be guarded against.
In user code, you should not generally check this counter - it is an internal counter used by the Sling system to enforce the limit. Nevertheless you can catch an the exception which AFAIK is not a really public one.
The second situation may potentially also be a problem, such as in an endless loop. But the limit for this kind of counter should be much higher than 50.
I think Bertrand wrote integration tests for this functionality. But these test did not include includes in succession.
> Request Inclusion Counter operating too rigid
> ---------------------------------------------
>
> Key: SLING-601
> URL: https://issues.apache.org/jira/browse/SLING-601
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Engine 2.0.2
> Reporter: Felix Meschberger
> Assignee: Felix Meschberger
> Fix For: Engine 2.0.4
>
>
> The SlingMainServlet implements a recursion level counter to prevent recursions going too deep in the checkRecursionLevel method. What this method does is incrementing a counter stored as a request attribute everytime RequestDispatcher.include is called (by whatever means).
> The drawback of this mechanism is, that the counter is only incremented and never decremented after an inclusion returns. This in fact is not a recursion level counter but a counter limiting the number of times RequestDispatcher.include may be called. Another drawback of this is, that the implementation is overcomplicated, because the RequestData object created for each request keeps a stack of a inclusions and hence could easily implement a real recursion level checker by just checking the size of the stack.
> I think, we should drop the SlingMainServlet.checkRecursionLevel method altogether and replace it with a new RequestData.checkAbort method which throws in case any of two cases occurrs:
> * Recursion Level is too deep (50 seems like useful)
> * Number of includes exceeds a given number (something like 1000 or more ?)
> In addition the Exception should be made public in the Sling engine module such that users of any include mechanism could handle it if need be.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-601) Request Inclusion Counter operating
too rigid
Posted by "Janandith Uditha Jayawardena (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625432#action_12625432 ]
Janandith Uditha Jayawardena commented on SLING-601:
----------------------------------------------------
How can I use the checkRecursionLevel method in code to check this problem.
should I include a script in 51 files recursively first.
ex: a ------ includes ------> b ------ includes -------> c .... etc.
secondly
should I call include 51 times in the same script.
ex: a ---->include b
---->include c
----- etc
How can I check the result for a failure. Is it by finding whether InfiniteIncludeLoopException is thrown.
> Request Inclusion Counter operating too rigid
> ---------------------------------------------
>
> Key: SLING-601
> URL: https://issues.apache.org/jira/browse/SLING-601
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Engine 2.0.2
> Reporter: Felix Meschberger
> Assignee: Felix Meschberger
> Fix For: Engine 2.0.4
>
>
> The SlingMainServlet implements a recursion level counter to prevent recursions going too deep in the checkRecursionLevel method. What this method does is incrementing a counter stored as a request attribute everytime RequestDispatcher.include is called (by whatever means).
> The drawback of this mechanism is, that the counter is only incremented and never decremented after an inclusion returns. This in fact is not a recursion level counter but a counter limiting the number of times RequestDispatcher.include may be called. Another drawback of this is, that the implementation is overcomplicated, because the RequestData object created for each request keeps a stack of a inclusions and hence could easily implement a real recursion level checker by just checking the size of the stack.
> I think, we should drop the SlingMainServlet.checkRecursionLevel method altogether and replace it with a new RequestData.checkAbort method which throws in case any of two cases occurrs:
> * Recursion Level is too deep (50 seems like useful)
> * Number of includes exceeds a given number (something like 1000 or more ?)
> In addition the Exception should be made public in the Sling engine module such that users of any include mechanism could handle it if need be.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-601) Request Inclusion Counter operating
too rigid
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625637#action_12625637 ]
Felix Meschberger commented on SLING-601:
-----------------------------------------
> endless loops are not easy detectable. there is nothing you can do to prevent people writing a "while(true);"
I was talking about endless inclusion loops of the form "while (true) { include("something") }". This form of endless loop can be detected by an inclusion counter, too. But as I said, this limit should be much higher, such as to allow big inclusions (like listing and displaying children) but stopping ridiculous inclusions (like inclusion loops with bugs like forgetting to increment a counter).
> Request Inclusion Counter operating too rigid
> ---------------------------------------------
>
> Key: SLING-601
> URL: https://issues.apache.org/jira/browse/SLING-601
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Engine 2.0.2
> Reporter: Felix Meschberger
> Assignee: Felix Meschberger
> Fix For: Engine 2.0.4
>
>
> The SlingMainServlet implements a recursion level counter to prevent recursions going too deep in the checkRecursionLevel method. What this method does is incrementing a counter stored as a request attribute everytime RequestDispatcher.include is called (by whatever means).
> The drawback of this mechanism is, that the counter is only incremented and never decremented after an inclusion returns. This in fact is not a recursion level counter but a counter limiting the number of times RequestDispatcher.include may be called. Another drawback of this is, that the implementation is overcomplicated, because the RequestData object created for each request keeps a stack of a inclusions and hence could easily implement a real recursion level checker by just checking the size of the stack.
> I think, we should drop the SlingMainServlet.checkRecursionLevel method altogether and replace it with a new RequestData.checkAbort method which throws in case any of two cases occurrs:
> * Recursion Level is too deep (50 seems like useful)
> * Number of includes exceeds a given number (something like 1000 or more ?)
> In addition the Exception should be made public in the Sling engine module such that users of any include mechanism could handle it if need be.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-601) Request Inclusion Counter operating
too rigid
Posted by "Carsten Ziegeler (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625645#action_12625645 ]
Carsten Ziegeler commented on SLING-601:
----------------------------------------
Yes, I think having both limits makes sense - as pointed out, the current mechanism checks the recursion level where a value of 50 should really be sufficient.
In addition couting the total number of includes and limiting them (to 1000) makes sense.
However, it would be great to have this configurable - I can imagine that if you're building heavy portal sites the limits should be higher.
> Request Inclusion Counter operating too rigid
> ---------------------------------------------
>
> Key: SLING-601
> URL: https://issues.apache.org/jira/browse/SLING-601
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Engine 2.0.2
> Reporter: Felix Meschberger
> Assignee: Felix Meschberger
> Fix For: Engine 2.0.4
>
>
> The SlingMainServlet implements a recursion level counter to prevent recursions going too deep in the checkRecursionLevel method. What this method does is incrementing a counter stored as a request attribute everytime RequestDispatcher.include is called (by whatever means).
> The drawback of this mechanism is, that the counter is only incremented and never decremented after an inclusion returns. This in fact is not a recursion level counter but a counter limiting the number of times RequestDispatcher.include may be called. Another drawback of this is, that the implementation is overcomplicated, because the RequestData object created for each request keeps a stack of a inclusions and hence could easily implement a real recursion level checker by just checking the size of the stack.
> I think, we should drop the SlingMainServlet.checkRecursionLevel method altogether and replace it with a new RequestData.checkAbort method which throws in case any of two cases occurrs:
> * Recursion Level is too deep (50 seems like useful)
> * Number of includes exceeds a given number (something like 1000 or more ?)
> In addition the Exception should be made public in the Sling engine module such that users of any include mechanism could handle it if need be.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.