Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Redirect.php - Fix #16 #20

Merged
merged 1 commit into from
Apr 29, 2018
Merged

Update Redirect.php - Fix #16 #20

merged 1 commit into from
Apr 29, 2018

Conversation

xiaohutai
Copy link
Collaborator

Not sure if this is the proper fix, because it seems a bit hacky to me.

@xiaohutai
Copy link
Collaborator Author

xiaohutai commented Dec 1, 2017

Wait, it seems this error only occurs when you haven't triggered something first. Because it's not happening anymore 🤔

I still need to familiarize with this extension.

@SvanteRichter
Copy link
Collaborator

@xiaohutai You mean you can't repro the original issue?

@xiaohutai
Copy link
Collaborator Author

The error happened on a clean install, but sometime later it never happened anymore. Not sure what's going on.

@xiaohutai
Copy link
Collaborator Author

Hm, maybe I didn't saved it correctly when it didn't occur.

This fix should work, but not sure if it is proper.

@artggd
Copy link
Contributor

artggd commented Mar 1, 2018

To repro the original issue, you'll need to enter this condition. Having no redirect in your request and no redirects: {login: /auth/profile} in app/config/extensions/auth.boltauth.yml.

I will provide a fix for the abovementioned condition in another PR soon. This one could still be merged I guess, forcing a __toString shouldn't hurt 🙂

@SvanteRichter
Copy link
Collaborator

This should have been fixed by #22, but let me know if it isn't and I'll merge this one too.

@SvanteRichter
Copy link
Collaborator

Reopening since it might fix an issue reported on slack.

@SvanteRichter SvanteRichter reopened this Apr 29, 2018
@SvanteRichter SvanteRichter merged commit 6997f2b into BoltAuth:master Apr 29, 2018
@SvanteRichter
Copy link
Collaborator

Seems to fix the other redirect problem not fixed by #22, so merging :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants