Skip to content
This repository has been archived by the owner on Feb 22, 2024. It is now read-only.

Maximum recursion depth exceeded #340

Closed
eriktaubeneck opened this issue Nov 14, 2014 · 15 comments
Closed

Maximum recursion depth exceeded #340

eriktaubeneck opened this issue Nov 14, 2014 · 15 comments
Labels

Comments

@eriktaubeneck
Copy link

If the security object gets initialized before the app exists (i.e. using the factory method) and init_app is called later, security._state is never set. This makes sense because _state is dependent upon the app, however, when an attribute on security is accessed, the redefinition of __getattr__ looks for _state, which isn't set, so __getattr__ get's called, looks for _state, etc, and then bam

RuntimeError: maximum recursion depth exceeded while calling a Python object

I opened a PR, but closed it because I'm not sure if it's the right fix. I solved the issue myself by doing:

security = app.extensions['security']
@security.login_context_processor
...

but at the very least, I think there should be a better exception that gets raised (the max recursion depth really threw me off for a bit.) The solution in the PR would work, but would require the above to become something like

with app.app_context():
    @security.login_context_processor
    ...

which maybe works? Curious what other people think is a good solution.

@eriktaubeneck
Copy link
Author

Looks like this is a common issue. (I searched for the recursion depth and nothing came up.) @mattupstate I can reopen that PR which will at least give a more appropriate error message that you're working outside of the current app context, or we could just raise a custom exception explicitly? Thoughts?

@dwcaraway
Copy link

+1

@bbenne10
Copy link

Is there a reason that you simply couldn't do self._state = state just before return state in core.py's init_app? I've got a fork that does just that, and it seems to resolve this issue (though not the one I was trying to resolve...)

@mcs07
Copy link

mcs07 commented Jan 20, 2015

I get this recursion problem when using @security.send_mail_task after doing a security.init_app() in my Flask application factory. @bbenne10's self._state = state solution seems to fix it for me.

@eriktaubeneck
Copy link
Author

@bbenne10 @mcs07 the issue with setting self._state = state is that security.init_app is designed to be used with the Flask factory pattern, and an instance of security needs to be able to work with multiple instances of a Flask app. app.extensions['security'] returns the state which is associated with the specific instance of app, which is what allows my above solution to work.

@nfvs
Copy link
Contributor

nfvs commented Feb 13, 2015

@bbenne10 I have a pull request pending #255 that does exactly that (besides other things), not sure when/if it will be merged though.

@eriktaubeneck
Copy link
Author

@nfvs it won't be. see this comment from last year. the fact that self._state doesn't get set to state in the factory method is intentional.

@nfvs
Copy link
Contributor

nfvs commented Feb 13, 2015

@eriktaubeneck awesome lookup skills, I have modified my PR accordingly :)

@eriktaubeneck
Copy link
Author

👍

@davvid
Copy link

davvid commented Dec 27, 2015

It seems like the one-line fix could instead be:

class Security(object):
    _state = _security  # add this line

That will make __getattr__ work for folks using the factory pattern since it'll proxy through the current application's security extension.

@diginikkari
Copy link

+1
At least there should be documentation what is correct way to use security extension when using flask factor pattern. Many of the extensions are used like this:

extension = Extension()
def create_app(config_filename):
    app = Flask(__name__)
    extension.init_app(app)

With Flask-Security there will be infinite loop with getattr when using this pattern.

I was able to get it working this way:

security = None
def create_app(config_filename):
    global security
    app = Flask(__name__)
    security = Security(app)

this could be stupid way, but at least it is working for me.

@KeNaCo
Copy link

KeNaCo commented Mar 16, 2016

As @eriktaubeneck and @diginikkari said, Flask-Social now dont support flask factory pattern. Also there is manual for it.
As I see some people try to fix it partially: #471, #465, #384(I ask @Diaoul if he want to fix failing tests or not. If not, I add this part to my solution.), #317
Also I start implement proper solution in my branch, pls review code and give me feedback.
State of work:

  • Implement flask factory pattern support DONE
  • (Old) Test failing for some python versions. - I think this is about my tox and py versions setup not about real bugs. DONE
  • Change tests to test factory approach. WIP

[Update 1] Old test: it was my setup problem, now tests fail only for py33, some _ssl3 stuff, w8 for package update.
[Update 2] Old test: yes, it was problem in py33 compilation. Now all test pass.
[Update 3] PM #495

@Richard-Mathie
Copy link

👍 this hits us too

for the time being we have to

security._state = security.init_app(app, user_datastore)

when we init, and

security.context_processor(security_context_processor)

instead of using the decorator patten

@jxltom
Copy link
Contributor

jxltom commented Feb 9, 2017

I have searched for the old issues and found that the author has explained here and here in #141 that the constructor and init_app are not meant to be used together. This is for initialize many applications with same Security instance. So this may not be a bug or just adding self._state = None as followings can fixing this issue.

if app is not None and datastore is not None:
    self._state = self.init_app(app, datastore, **kwargs)
else:
    self._state = None

@diginikkari The proper way for initialize is that accessing attributes by getattr is OK when using constructors. But when using factory method, users need to access security attributes by state object which is returned by init_app. Here is an example on how to initialize using factory method. This example is for accessing send_mail_task, but you can use it for other attributes such as login_context_processor as well. I have posted a more detailed example on how to use Celey async task as send_mail_task using factory method here.

from celery import Celery
from flask import Flask
from flask_security import Security, .. datastore stuff ..
from flask_mail import Mail

celery = Celery()
mail = Mail()
security = Security()

def create_app(config):
    """Initialize Flask instance."""
    app = Flask(__name__)
    mail.init_app(app)
    security_ctx = security.init_app(app,..datastore stuff..)

    @security_ctx.send_mail_task
    def delay_flask_security_mail(msg):
        send_flask_mail.delay(msg)

    return app

@krbullock
Copy link

Looks like this was fixed in #703.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests