-
-
Notifications
You must be signed in to change notification settings - Fork 533
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
Upgrade from commit cdf1327 to 922455c fails: PHP Parse error: syntax error, unexpected ')' #1856
Comments
The newest commit fixes this. |
I really hate to say this (I'm sure you hate hearing it even more), but with 4d23830 I'm seeing exactly the same behaviour: It freezes right after I provide a valid path to my SaltKey directory, with the same "syntax error, unexpected ')'" error. My tp.config.php is again unpopulated. I reviewed your change in 4d23830 and verified that my upgraded code contained the $ldap_suffix change. So I believe I'm on the correct code. To be thorough, I also tried another fresh install of 4d23830, with an empty SaltKey directory and empty database. The install completed, however after removing the install/ subdirectory I was still unable to login. If I repeated my login attempt several times it gave me the 10-second warning. The only log info emitted each time I attempted a login was this:
My tp.config.php (for my fresh-install test) ended up being as follows:
|
My mistake, I've confused your case with something else. So I think I have fixed your case, so that at least this error is not any more. If this happens again, Can you please run next query in mysql?
|
I see that commit 77e75c9 has appeared. I'll test that after I've completed this response. Regarding root-cause: I've made before-and-after screenshots of my 'teampass_misc' table, showing: 1) Contents on a working instance pre-upgrade, and 2) Contents after the upgrade freezes. I notice one difference: after the upgrade, row ID 25 "encryptClientServer 1" exists. That row did not exist before the upgrade. I'm not sure what this value implies, but if it matters, I am running my webserver over https with a valid (LetsEncrypt) cert, not a self-signed one. Here are the two screenshots: Before UpgradeAfter Upgrade |
I did some more upgrade tests from my working cdf1327, to both 77e75c9 and the newest (as of Aug 16th) 7e0875a. Both failed with exactly the same error as was originally described in this issue. I also tested a fresh install of the latest 7e0875a. It also failed the same way as described in my comment on Aug 10th. E.g. The installation appeared to successfully complete (although I had to manually delete the install/ directory. However I'm unable to log in. for this fresh-install test my _misc table is populated with 25 rows, and my tp.config.php file is populated with 117 lines. The only httpd log output each time I try to log in is:
Something really weird is going on here. I'd suspect that my environment is hosed, were it not for the fact that, despite the fact I'm using https (with a valid cert) and a few symlinks here and there, everything worked fine up to cdf1327. I'm a coder, but not a php/javascript coder, and actually not a webapp coder at all. Nevertheless I'll see if I can get Xdebug working here, and perhaps derive some better insight into what's going on. |
After debugging a fresh install of your latest commit 77e75c9, I've found at least two issues: The code refers to a file called "includes/libraries/protect/AntiXSS/AntiXss.php". However the actual filename is AntiXSS.php (note the capitalized SS). This causes identify.php (line 280) to fail its require_once(), and it appears to fail silently without any httpd logging output. This happens in more than one place:
After fixing that, I hit the 2nd problem: When installing a new instance of commit 77e75c9, it looks like the first row of table _user (which presumably should contain the newly-created admin user) is not populated. Thus, identify.php (starting line 659) fails its DB::queryFirstRow( ... ). I have no idea why the row isn't populated. The error printed on the webpage (Login credentials do not correspond!) could probably be changed a bit to more clearly indicate missing (not mismatching) credentials. Hopefully this gives you a bit more detail to go on.... |
Thank you for the investigation and restitution. |
I did a fresh-install test on your latest commit 716c1b8. The AntiXSS.php problem looks fixed. Thank you very much. The issue with the _user table not being populated with an initial admin user still exists. If I manually populate it with valid encrypted credentials, then I'm finally able to log in and reach the initial Settings screen - so that might be the last issue obstructing a successful fresh install. I have suspended testing the upgrade path from cdf1327 to current until the fresh-installation scenario becomes stable. Thanks again! |
This is really strange. |
I have found what I am doing to trigger the problem. Past experience has taught me that once the install is finished, the app will fail to remove the install/ folder with the following apache error:
So, I must manually delete the folder, which isn't a big problem. However because of this I have been skipping a step. In the following screen shot, there is "Move to home page" link: I have not been clicking this. Instead I've been flushing cache in my browser, closing it, deleting install/ manually, restarting Apache, and then opening a new browser instance and surfing to TeamPass. This is what is causing breakage. If instead, I DO click on that link (and then delete install/, flush cache and restart) it seems my _users table gets populated with 3 rows: I can then log in successfully (yay!). So, it looks like that link isn't just a redirect to the main TP page. It actually does something to (at least) the _users table. That Thank You page gives the impression that everything is done (written to file, committed to DB, etc.), and all that remains is to surf to the main TP page. Perhaps the wording of that page should be altered to warn users that they actually need to click that last link or things won't work. Thanks very-much for your patience. |
Thank you, Nils. Apparently I need to drink more coffee before I repeat older tests, because of-course you are right - I am now seeing that the _users table gets populated earlier on in the installation process. I have tried to recreate the failure I was seeing a few days ago, and I cannot. So either I didn't drink enough coffee, or my environment was messed up. So to summarize - I can now successfully do a fresh install of latest commit 716c1b8, and afterward, I can successfully log in with my new admin user. The PHP Fatal error: Uncaught Error: Call to undefined function chmodRecursive() does still happen though - it is very repeatable. So I do still need to manually delete/move/rename the install/. However I will open a separate issue for that to make things easier for you to track. I also still cannot upgrade from commit cdf1327 to anything newer - I still end up with an unpopulated includes/config/tp.config.php file. However I don't know if that's really a worthwhile problem anymore, since it's an upgrade from a development commit, not from a release. With your permission I'll close this issue. Thanks again for your help and patience. |
You are welcome. Nevertheless I don't understand why the script doesn't remove the My tests are running on Windows computer, perhaps are you on Linux. Can you confirm your OS? |
Linux - Fedora 26, and Apache 2.4. I've opened #1881 to track this. |
Steps to reproduce
Expected behaviour
Upgrade should complete, followed by successful login to an administrator account.
Actual behaviour
Upgrade reaches Step 5 - Miscellaneous screen. On this screen, the absolute path to an existing SaltKey directory is provided. Once LAUNCH is clicked, "Please Wait" is displayed with a partial swirl, which then freezes. No further output is displayed indefinitely.
Server configuration
Operating system: Fedora 26 (x86_64)
Web server: Apache 2.4.27
Database: MariaDB 10.1.25
PHP version: 7.1.7
Teampass version: cdf1327, upgrading to 922455c
Teampass configuration file:
Updated from an older Teampass or fresh install: cdf1327, upgrading to 922455c
tp.config.php
Client configuration
Browser: 54.0.1 (64-bit)
Operating system: Windows 10 (64-bit)
Logs
Web server error log
Firebug log (How to?)
The text was updated successfully, but these errors were encountered: