[ https://issues.apache.org/jira/browse/SLING-13353 ]
Rishabh Daim deleted comment on SLING-13353:
--------------------------------------
was (Author: JIRAUSER299730):
h3. AI-reviewed assessment
*Disclaimer:* This comment was prepared with AI assistance and should be
confirmed by the project maintainers.
I reviewed the [MAIA
review|https://github.com/apache/sling-org-apache-sling-auth-core/pull/29#pullrequestreview-5439354322]
against the merged implementation.
*Medium finding — marker lifetime:* I could not validate this as a concrete
issue in the supported authentication flow. {{handleSecurity}} returns early
when a resolver is already attached to the request, and successful anonymous
resolution stores that resolver on the request. I found no in-repository path
where a normal include or forward reaches {{handleLoginFailure}} again after
successful resolution. The concern remains a theoretical edge case if a caller
re-enters authentication after removing the resolver attribute. I do not
recommend a production change for the merged PR without an integration test
reproducing that lifecycle.
*Low finding — test structure:* This is a valid optional test-quality
suggestion. The regression test uses private reflection, consistent with
existing tests, and verifies the key failure mode: one anonymous login attempt
with no recursion. It could be strengthened by asserting the terminal error or
login-response behavior, but this is not a production defect.
*Verification:* The targeted tests passed 40/40 on Java 17; the full suite had
previously passed 304 tests. Repository history also showed no earlier fix for
this exact recursion; the older {{ReadOnlyAuthenticationInfo.stackGuard}}
addresses a separate issue.
No corrective action is required based on this review. A follow-up integration
test could be added if the project wants to validate re-entry after resolver
cleanup.
> SlingAuthenticator: unbounded handleLoginFailure <-> getAnonymousResolver
> recursion when anonymous login repeatedly fails
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: SLING-13353
> URL: https://issues.apache.org/jira/browse/SLING-13353
> Project: Sling
> Issue Type: Bug
> Affects Versions: Auth Core 2.0.2
> Reporter: Rishabh Daim
> Priority: Major
>
> h4. Problem
> {{SlingAuthenticator}} recurses without bound between {{handleLoginFailure}}
> and {{getAnonymousResolver}} when acquiring the anonymous
> {{ResourceResolver}} keeps failing with {{LoginException}} (e.g. the backing
> repository is unavailable). The stack grows until {{StackOverflowError}}, and
> since each failed request logs the full repeating trace, a sustained failure
> floods the error log (observed in the field growing until the disk filled).
> h4. Cause
> In {{org.apache.sling.auth.core.impl.SlingAuthenticator}}, the two methods
> call each other with no termination guard:
> * {{getAnonymousResolver}}: on {{LoginException}} from
> {{getResourceResolver}}, calls {{handleLoginFailure}};
> * {{handleLoginFailure}}: for a {{LoginException}} with anonymous access
> still allowed, calls {{getAnonymousResolver}}.
> So {{getAnonymousResolver -> handleLoginFailure -> getAnonymousResolver ->
> ...}} repeats as long as the anonymous login keeps failing. The anonymous
> fallback is never marked as already-attempted.
> h4. Reproduce
> Anonymous access allowed; make {{getResourceResolver}} throw
> {{LoginException}}; issue a plain request to an anonymous path. Expected: one
> anonymous attempt, then terminate. Actual: recursion to
> {{StackOverflowError}}.
> h4. Fix
> Per-request re-entrancy guard: {{getAnonymousResolver}} marks (request
> attribute) that anonymous resolution was attempted; {{handleLoginFailure}}
> only falls back when the mark is absent - at most one attempt per request,
> terminating normally on repeated failure. All currently-successful paths
> (including a single legitimate anonymous fallback) are unchanged. Regression
> test drives repeated {{LoginException}} and asserts no {{StackOverflowError}}
> and exactly one {{getResourceResolver}} call; it fails without the guard.
> Affects current trunk (auth-core bundle 66) and earlier releases with the
> same structure.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
