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

Redirect loop on fresh lane install #976

Closed
scannerdarkly opened this issue Mar 1, 2018 · 9 comments
Closed

Redirect loop on fresh lane install #976

scannerdarkly opened this issue Mar 1, 2018 · 9 comments

Comments

@scannerdarkly
Copy link
Contributor

scannerdarkly commented Mar 1, 2018

Just did a fresh install: Ubuntu 16.04.4 LTS / nginx 1.10.3 / mysql 5.7.21 / PHP 7.0.25, and CORE-POS 2.7. http://lane2/lane/install seemed to go perfectly.

http://lane2/lane/ triggers a redirect loop between /lane/login.php and /lane/gui-modules/login2.php:

  • /lane/login.php redirects to /lane/gui-modules/login2.php because CoreLocal::get('LoggedIn') == 0.
  • /lane/gui-modules/login2.php redirects back to /lane/login.php because it calls AutoLoader::dispatch() (whose parameter $redirect defaults to true), and CoreLocal::get('CashierNo') = 6000 and $class = '' which triggers the redirect.

I’ve never logged in as CashierNo 6000, but I do see that value in globalvalues, so something seems to have set it this way.

I’m fine with tweaking the database to break us out of this loop, but unless I’m missing something this seems like a bug.

  • Version of CORE?
    2.7

  • Issue with Office, Lane, or both?
    lane

  • Is this [mostly] a bug report, feature request, or question?
    bug report

@gohanman
Copy link
Contributor

gohanman commented Mar 1, 2018

How is $class an empty string? It should be login2. Maybe you're missing fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name; in your nginx config?

gohanman added a commit that referenced this issue Mar 1, 2018
It doesn't *do* anything besides die and log an error.
There's not realy a recovery option
gohanman added a commit that referenced this issue Mar 1, 2018
I broke the environment manually to verify the appropriate
error was logged, but I need to un-break it again to make
everything actually work
@scannerdarkly
Copy link
Contributor Author

/etc/nginx/fastcgi.conf does indeed contain fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;.

$_SERVER['PHP_SELF'] is blank, however. Googling “nginx PHP_SELF”, this seems to be a frequent issue with nginx — a lot of people tearing their hair out about this one over the years.

I’m not totally opposed to tearing some of my own hair out getting nginx to support PHP_SELF, but I’m wondering if it might be best overall to simply change CORE-POS to use one of:

  • $_SERVER['SCRIPT_NAME'] (URL relative to root, not including any ?name=value parameters)
  • $_SERVER['SCRIPT_FILENAME'] (full filesystem path, leaves off parameters)

Both seem to be supported under both Apache and nginx. SCRIPT_NAME, particularly, seems like it’ll work identically in any real-world CORE-POS setup (see here for some discussion of the few situations where they would differ).

Any chance we could do that?

@gohanman
Copy link
Contributor

gohanman commented Mar 1, 2018

We could, but PHP_SELF is more widely used than just this one spot. Googling "php script_name empty" turns up problem posts, too, so it's probably not a bulletproof solution either.

The scope of the problem is bigger on the backend where PHP_SELF is used hundreds of times.

@scannerdarkly
Copy link
Contributor Author

I can confirm that changing PHP_SELF to SCRIPT_NAME in AutoLoader::dispatch() fixes everything and lets me log in.

@scannerdarkly
Copy link
Contributor Author

Ah, the joy of “standards”!

gohanman added a commit that referenced this issue Mar 1, 2018
@gohanman
Copy link
Contributor

gohanman commented Mar 1, 2018

^ That seems somewhat more robust

@scannerdarkly
Copy link
Contributor Author

Thank you! I suppose there’s no chance of a backport to 2.7...

@gohanman
Copy link
Contributor

gohanman commented Mar 1, 2018

That doesn't cover everything; it'd have to be a larger backport

gohanman added a commit that referenced this issue Mar 1, 2018
This commit is the non-plugin usage
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
gohanman added a commit that referenced this issue Mar 1, 2018
@gohanman
Copy link
Contributor

gohanman commented May 9, 2018

Fixed in 2.9.0

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

No branches or pull requests

2 participants