Call to a member function beginTransaction() on a non-object

[PineSmokes]

Trying to finish up this PHP section to this login and sign up section.  At the moment I'm having issues with the sign up, it's throwing me an error when I signed up, not sure exactly what happened because it did work once and only once.

My error throws on line 206 the beginTransaction line.

 

http://www.golden-wand.com/members/contact-test.php

				try{
					$db->beginTransaction();
					$ipaddress = getenv('REMOTE_ADDR');
					$stmt2 = $db->prepare("INSERT INTO members (firstname, lastname, username, email, password, signup_date, ipaddress) VALUES (:fistname, :lastname, :username, :email, :bcrypt, now(), :ipaddress)");
					$stmt2->bindParam(':fistname',$fistname,PDO::PARAM_STR);
					$stmt2->bindParam(':lastname',$lastname,PDO::PARAM_STR);
					$stmt2->bindParam(':username',$username,PDO::PARAM_STR);
					$stmt2->bindParam(':email',$email,PDO::PARAM_STR);
					$stmt2->bindParam(':bcrypt',$bcrypt,PDO::PARAM_STR);
					$stmt2->bindParam(':ipaddress',$ipaddress,PDO::PARAM_INT);
					$stmt2->execute();
					/// Get the last id inserted to the db which is now this users id for activation and member folder creation ////
					$lastId = $db->lastInsertId();
					$stmt3 = $db->prepare("INSERT INTO activate (user, token) VALUES ('$lastId', :token)");
					$stmt3->bindValue(':token',$token,PDO::PARAM_STR);
					$stmt3->execute();
					// Create our email body
						$link = 'http://golden-wand.com/Scripts/activate.php?user='.$lastId.'&token='.$token.'';
						$data = "Thanks for registering an account at Golden Wand! We are glad you decided to join us.
								Theres just one last step to set up your account. Please click the link below to confirm your identity and get started.
								If the link below is not active please copy and paste it into your browser address bar.
								<br><br>
								$link";
					// Create the Transport
						$transport = Swift_SmtpTransport::newInstance('smtp.gmail.com', 465, 'ssl')
							 ->setUsername($user_name)
							 ->setPassword($pass_word);
					// Create the Mailer using your created Transport
						$mailer = Swift_Mailer::newInstance($transport);
						// Create a message
						$message = Swift_Message::newInstance('Sign Up')
						  ->setFrom(array('support@golden-wand.com' => 'From: Auto Resposder @ Golden Wand'))
						  ->setTo(array('ditto.100@gmail.com' => 'Recipient'))
						  ->setSubject('IMPORTANT: Activate your Golden Wand Account')
						  ->setBody($data, 'text/html')
						  ;
						// Send the message
						$result = $mailer->send($message);
			
					$db->commit();
					$msg .= "<li class='success'>Thanks for joining! Check your email in a few moments to activate your account so that you may log in. See you on the site!</li>";
					$db = null;
				}
				catch(PDOException $e){
					$db->rollBack();
					echo $e->getMessage();
					$db = null;
				}

I own this site http://www.golden-wand.com/phpfreaks.txt

[Jacques1]

Your $db variable is not a PDO connection. Maybe the variable name is wrong, maybe you haven't even included your database script, maybe the connection failed but you keep going nonetheless.

 

By the way, is there any reason why you need to store the activation token in a separate table? The code would be a lot easier (no second query, no transaction), if you stored the token right in the members table.

[PineSmokes]

Pretty sure it's a PDO connect, sorry I forgot to post it here's the file. It is easier and from what I've learned more secure, if I keep keys and passwords in a separate file that is forbidden to access, seems secure to me and it was working at one point but I deleted the page I had completed on accident now I'm having issues remaking it is all.

<?php
include 'smconfig.php';
$db_host = "127.0.0.1";
$db_username = "root";
$db_pass = "$dbpass";
$db_name = "golden_wand";

//  PDO  CONNECT
	$db = new PDO('mysql:host='.$db_host.';dbname='.$db_name,$db_username,$db_pass);
	$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

The password is in the include file since it's my secure password, this connect works I have thrown a try catch in there to test and it came back successful.  Is there something wrong with it?

 

Edit:

Ahh activation token in a seaprate file, I was learning PHP and PDO a week or so ago from Isaac Price on his YouTube channel, he had some very very helpful videos and taught me quite a bit about the login process.  When I was learning there some of those files are still separate like he had them.  He had literally everything as a separate file it seemed I like to try to be a little more in one file but I'm still developing it and learning at the same time.

 

Here's the link to his files, no need to post his YouTube video I'm sure you know enough but if anyone want to find it, his name will be enough.

https://github.com/IsaacNeal/backburnr

 

He was the entire reason I switched from the other Mysqli connect to this PDO version, it was obvious why PDO is necessary after watching his videos

Edited November 16, 2015 by PineSmokes

[Jacques1]

Pretty sure it's a PDO connect [...]

 

Well, PHP disagrees. Are you sure you've actually included the database script in the main script?

 

 

 

[...] it was working at one point but I deleted the page I had completed on accident [...]

 

You should use version control software like Mercurial or Git. This allows you to save snapshots of your code and restore previous versions in case you accidentically mess up your application.

 

 

 

Ahh activation token in a seaprate file [...]

 

You mean a separate table, right?

 

The problem with storing the token in a separate table is that you need two queries instead of one, and you additionally need a transaction to turn the two queries into a single logical action.

 

Why make things so complicated? There's a one-to-one relationship between the token and the member, so you can and should simply put the token into the members table.

 

 

 

Here's the link to his files, no need to post his YouTube video I'm sure you know enough but if anyone want to find it, his name will be enough.

https://github.com/IsaacNeal/backburnr

 

He was the entire reason I switched from the other Mysqli connect to this PDO version, it was obvious why PDO is necessary after watching his videos

 

The code you're learning from is relatively good compared to most other PHP tutorials, but it's far from perfect. Many parts are nonsensical or even harmful (like using strip_tags() or printing database errors on the screen or relying on some homegrown password hash mechanism).

 

So I don't think it's a good idea to just copy and paste the entire code. Learn from multiple resources, take everything with a grain of salt, and write your own code.

[PineSmokes]

It's totally included, the file is at the top of my script and Dreamweaver see's all of the included files as well like in the attached image.  Also I did a simple try catch to do what I think is a test, you tell me if this works or not, it's outputting the entire string on to of the page which looks to me like a success.

http://www.golden-wand.com/members/contact-test.php

try {
	$db = new PDO('mysql:host='.$db_host.';dbname='.$db_name,$db_username,$db_pass);
    foreach($db->query('SELECT * from members') as $row) {
        print_r($row);
    }
    $db = null;
} catch (PDOException $e) {
    $msg .= "<li class='error'>$e->getMessage()";
    $db = null;
}

I definitely did my best to write it up myself no copying that way I was learning something in the process, then I tried to pull it all together and make something on this test page instead of just a black page and that's when it got a little dicey.  I hashed with sha512 once everything was working I planned to maybe hash it better since there are better options.  If there are better methods for handling the password could you point me in that direction.  I redid the entire script to make it better and more secure, that's my goal, I don't expect to get it right the first time around but I did think I was doing something better than "homegrown" password mechanics, like where do I go to learn about professional password mechanics hah

 

I'll be attempting to move the activation column token to the members table at some point since that indeed does make very little sense.  Also why are strip_tags harmful?  I thought it takes the spaces out of the front and end of a users input in case they accidentally ended with a space or maybe even started with one, if there's a better way or if this is bad practice I would like to learn the correct way if you could

 

Is this what you meant by nonsensical or harmful?  The printing of the message?  I thought I was doing that because I'm developing and would like to see errors then remove later?

catch(PDOException $e){
     $msg .= "<li class='error'>$e->getMessage()</li>";
     $db = null;
}

You are very helpful and I overly appreciate the time you are taking to respond to my inquiries, so thank you very much

 

 

[Jacques1]

Now you're setting the $db variable to null after you've printed the test result. That's a bad idea, because now $db definitely isn't a PDO connection. Remove the assignments and try again.

 

 

 

I redid the entire script to make it better and more secure, that's my goal, I don't expect to get it right the first time around but I did think I was doing something better than "homegrown" password mechanics, like where do I go to learn about professional password mechanics hah

 

PHP has a password hashing API which hides all the low-level cryptography behind a few simple functions. To hash a password, all you have to do is call password_hash():

<?php

// password hash parameters; put this into a separate configuration file
const PASSWORD_HASH_ALGO = PASSWORD_BCRYPT;    // bcrypt is currently the only choice
const PASSWORD_HASH_COST = 14;                 // adjust this to your own hardware (hashing a password should take roughly one second)
const PASSWORD_MAX_LENGTH = 56;                // bcrypt has a maximum input length of 56 bytes



$test_password = 'zkWMfOmSTPS4wY8C8BzCaG';

// create a hash from the plaintext password
if (strlen($test_password) <= PASSWORD_MAX_LENGTH)
{
    $password_hash = password_hash($test_password, PASSWORD_HASH_ALGO, ['cost' => PASSWORD_HASH_COST]);
    echo 'The password hash is: '.$password_hash;
}
else
{
    echo 'The password must not be longer than '.PASSWORD_MAX_LENGTH.' bytes.';
}

To verify a password against a hash, you just have to call password_verify():

<?php

$password_hash = '$2y$14$5ue3w80sIho.B5GjqlppB.nwwnpUL3WN8re5peY3sRJ.w5idlgmKC';



$test_password ='zkWMfOmSTPS4wY8C8BzCaG';

if (password_verify($test_password, $password_hash))
{
    echo 'The password is correct.';
}
else
{
    echo 'The password is incorrect';
}

Also why are strip_tags harmful?

 

Because this function mangles the input in a primitive attempt to remove HTML tags. This is nonsensical and will lead to data corruption. For example, the perfectly valid string “1 < 3” will be truncated to “1 ” just because it happens to contain the “<” character.

 

Never change the user input. Store it as is. When you need to insert the data into a critical context, use escaping (e. g. htmlspecialchars() for HTML contexts).

 

 

 

Is this what you meant by nonsensical or harmful?  The printing of the message?  I thought I was doing that because I'm developing and would like to see errors then remove later?

 

There's no need to clutter your code with try statements and stuff like echo $e->getMessage(), because PHP itself is perfectly capable of printing error messages. Simply enable display_errors in your PHP configuration and set error_reporting to -1. Now all unhandled errors will be printed on the screen. If you want the message to be nicely formatted, you can use the Xdebug extension.

 

When the code goes into production, you turn display_errors off and enable log_errors instead. This is much safer than having to remove tons of try statements, because there's no risk of overlooking one of those debug statements and accidentally revealing information about your server.

 

In general, you should leave exceptions alone and let the PHP error handler do its job. Catching an exception is only needed if you have a specific solution to this specific error, which is rare. Most problems cannot be solved at runtime.

Edited November 17, 2015 by Jacques1

[PineSmokes]

Now you're setting the $db variable to null after you've printed the test result. That's a bad idea, because now $db definitely isn't a PDO connection. Remove the assignments and try again.

 

And you're amazing, not sure where I picked up that line of code, I think I didn't want a white screen with the exit(); code, what do I do to stop the script nicely when it realizes I already have that email address in the system or when it fails in general?  It works when I removed ALL the $db = null code except it obviously now doesn't know how to stop.  That was my whole issue I guess and I didn't even know it please let me know how to fix it then I'll go work on everything else you noted  Thanks so much I really appreciate your knowledge.

[PineSmokes]

And you're amazing, not sure where I picked up that line of code, I think I didn't want a white screen with the exit(); code, what do I do to stop the script nicely when it realizes I already have that email address in the system or when it fails in general?  It works when I removed ALL the $db = null code except it obviously now doesn't know how to stop.  That was my whole issue I guess and I didn't even know it please let me know how to fix it then I'll go work on everything else you noted  Thanks so much I really appreciate your knowledge.

 

I got it, I set $ok = "true" at the start of the posted section from the sign up button click.  Then wherever there was an error message I had written to the $msg variable (basically once inside every else{} or catch{} blocks) I set ok = "" (which made ok = !ok).  Last I set up an if(!ok){ echo "Not ok"; } else { processing code goes here } section and everything seems to stop correctly except it doesn't really have an end to the script.  Did I just use $db=null too early?  Should I still use it on the end of my PHP code after either the processing code or the final errors when the code is done?

Edited November 17, 2015 by PineSmokes

[Jacques1]

You shouldn't do anything with the database connection. Setting it to null at some point is very confusing and error-prone, because if you happen to need the connection afterwards (e. g. to roll back a transaction or log data), you'll again run into “non-object” errors. PHP itself will terminate the database connection after script execution has ended, so no need to do that manually.

 

You shouldn't use “magical strings” like "true" either, because this again leads to confusion and bugs. Yes, PHP's sloppy type system will accept expressions like !"true", but it won't necessarily do what you expect. For example, !"false" also evaluates to false, even though you probably wanted true.

 

Use actual booleans, and use meaningful variable names like $registration_successful rather than a generic $ok (which could mean pretty much anything). Even better: Separate the error handling from the rendering of error message. In your application code, you only collect the errors that happened. And at the very end, you render the message and send an appropriate response code. Don't forget the HTTP code! While most of us are good at processing purely visual information, this isn't true for every client (consider a visually impaired human or an automated client).

<?php

// possible errors
const ERROR_EMAIL_ADDRESS_EXISTS = 0;
const ERROR_EMAIL_TRANSMISSION_FAILED = 1;
// ...



// collect all errors in a set
$registration_errors = [];
$response_code = 200;

// example error: violation of UNIQUE constraint of email column
$registration_errors[ERROR_EMAIL_ADDRESS_EXISTS] = true;
$response_code = 400;

// set the HTTP response code
http_response_code($response_code);
?>
<!-- your HTML markup -->
<?php if (array_key_exists(ERROR_EMAIL_ADDRESS_EXISTS, $registration_errors)): ?>
    Sorry, but your e-mail address is already registered. Do you want to <a href="password_reset.php">reset your password</a>?
<?php endif; ?>