• Post Reply Bookmark Topic Watch Topic
  • New Topic

Session data getting not getting set  RSS feed

 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ok I've got a new implementation for my site. I've got an index servlet, a formController servlet, a Login.class/bean, and a custom made Session API.

Lets start with the index servlet. This servlet simply forwards the user to the index.jsp page with certain attributes. Attributes for users who aren't logged in and for users who are logged in. Messages get set via Sessions from one servlet/bean to another. The index servlet code:



When a user enters their login details the form data is submitted to the formController servlet which determines which form has been submitted. formController servlet code:




Lets say we're trying to login, the switch() statement will catch the the form attribute called FORM_SUBMIT parsed into a HashMap and it will call upon the Login.class/bean. Login code:



And finally my custom made Session API.



I know its a lot to take in but please give me the time for help. As its greatly appreciated. In a nutshell no session data is being set between the servlets and classes! Many thanks! I know the Session API works because ive tested it on a few test servlets  and JSPs. Many thanks! Took me ages to do this lol Cheers ppl!
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sorry the login code is:



Sorry lol..
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Unfortunately you've posted several hundred lines of code, so it's quite likely that I may have missed something while scrolling up and down. However... you create a new Session object in each of the two servlets. So if your first servlet put something into the Session object, the second servlet isn't going to see that because it creates another Session object.

 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:Unfortunately you've posted several hundred lines of code, so it's quite likely that I may have missed something while scrolling up and down. However... you create a new Session object in each of the two servlets. So if your first servlet put something into the Session object, the second servlet isn't going to see that because it creates another Session object.




Yeah i was thinking the same thing, Feel free to do any mods to the code cos I need this woking asap :( as my boss has set me a deadline :( Cheers, and many, MANY thanks!...

PS: I'm creating 2 different Session objects aren't I?
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If look at line 45 in the my formController serlvet I create a new Session object and im passing that in as an arg in line 37 in the Login bean. So im unsure whether theres acutally 2 Session objects being initialized..
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ive posted the same code for the formController servlet so be wary of this...
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well, I have two problems dealing with this question.

First, not enough information. You have two servlets and I can't tell which is run first and which is run in response to the JSP written by the first one.

Second, too much information. There's way too much code to go through.

Third, okay three problems then, you've written a Session class which looks like a wrapper for the session. It may or may not work properly. It would help to test it with a much less complicated pair of servlets.
 
Bear Bibeault
Author and ninkuma
Marshal
Posts: 66261
151
IntelliJ IDE Java jQuery Mac Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:Third, okay three problems then, you've written a Session class which looks like a wrapper for the session.

A good point. I've written dozens of web apps and never felt a need to do this. You might perhaps think about whether you're over-engineering things (which frequently leads to just adding more points of possible failure).
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:Well, I have two problems dealing with this question.

First, not enough information. You have two servlets and I can't tell which is run first and which is run in response to the JSP written by the first one.

Second, too much information. There's way too much code to go through.

Third, okay three problems then, you've written a Session class which looks like a wrapper for the session. It may or may not work properly. It would help to test it with a much less complicated pair of servlets.


Step 1: User visits index page/servlet.
Step 2. User enters his login details.
Step 3. The login form submits to the formController servlet decides which form we're dealing with, lets take login for an example.
Step 4. The formController creates a Login object to to process the users details. Then invokes the login(Map<sring, string> form data) method.
Step 5. If everything checks out the Loging bean will set the user's details as session vars using my Session API/Wrapper and returns true.
Step 7. formController redirects the user back to the index page/servlet logged in.

Yes I've testing my Session API on a few test servlets and it works fine!!
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Bear Bibeault wrote:
Paul Clapham wrote:Third, okay three problems then, you've written a Session class which looks like a wrapper for the session.

A good point. I've written dozens of web apps and never felt a need to do this. You might perhaps think about whether you're over-engineering things (which frequently leads to just adding more points of possible failure).


But it works absolutely fine tho. And I can use it for other projects too.
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Michael Portman wrote:Step 1: User visits index page/servlet.
Step 2. User enters his login details.
Step 3. The login form submits to the formController servlet decides which form we're dealing with, lets take login for an example.
Step 4. The formController creates a Login object to to process the users details. Then invokes the login(Map<sring, string> form data) method.
Step 5. If everything checks out the Loging bean will set the user's details as session vars using my Session API/Wrapper and returns true.
Step 7. formController redirects the user back to the index page/servlet logged in.


Okay. And at what stage does what actually happens diverge from this description?
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Michael Portman wrote:Step 1: User visits index page/servlet.


Sorry, I'm confused about what this means. Does the user visit a page which requests the index servlet?

Step 2. User enters his login details.


Into what page? One written by the index servlet, or does this login-details page request the index servlet?
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The index sevlet forwards to the index.jsp page for HTML representation which contains the login form. I've done a little mod to to the formController servlet which looks like this:



When I check the URL of /index it contains /index?true. So the login method is working and returning true but no session data is getting set.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Looks likes no one can help :-(. No worries I'll think of a different implementation. Thanks for trying though, much appreciated!
 
Dave Tolls
Ranch Foreman
Posts: 3009
37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Not gone through the code (there's too much for a quick look first thing on a Friday) but...you have state in a servlet.
Specifically your formController (which should, by Java naming standards, be FormController) store the request data as a HashMap as an attribute.  That is seriously prone to going wrong in a multi-threaded environment like a webapp.  If you have two people hitting the servlet at the same time, the second one could easily overwrite the formData object before the first one gets to the Login class constructor, resulting in the first user ending up logged in as the second user.  This would often be classed as a Bad Thing.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dave Tolls wrote:Not gone through the code (there's too much for a quick look first thing on a Friday) but...you have state in a servlet.
Specifically your formController (which should, by Java naming standards, be FormController) store the request data as a HashMap as an attribute.  That is seriously prone to going wrong in a multi-threaded environment like a webapp.  If you have two people hitting the servlet at the same time, the second one could easily overwrite the formData object before the first one gets to the Login class constructor, resulting in the first user ending up logged in as the second user.  This would often be classed as a Bad Thing.


So how do I do fix this, correct my code if you can please? Be easy on me as im relatively new to Java, im migrating from PHP.









 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
maybe put the hashmap inside in the doPost method?
 
Dave Tolls
Ranch Foreman
Posts: 3009
37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Michael Portman wrote:
So how do I do fix this, correct my code if you can please? Be easy on me as im relatively new to Java, im migrating from PHP.


Pretty much that.
But someone should be helping you through this if you're that new.  I would have expected a lot of pairing, instead of "here you go, fix this".
 
Dave Tolls
Ranch Foreman
Posts: 3009
37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
OK, looking at that Login class.

The use of that MySQL class seems to imply you are not using a connection pool, which is generally not a good idea with a webapp.  Depending on how many concurrent users you are likely to have you may find yourself running out of connections.
I'm going to treat the Login class like a DAO (Data Access Object), since that seems to be what it is.
So, when interacting with the database using JDBC in a webapp you'll generally see the following structure in a method:

You need those try-with-resources blocks to ensure the resources (the result set, statement and connection) are closed, otherwise you are going to run out.

The getConnection() call simply hides whatever mechanism you are using to get a connection...as I say above, it should be using a connection pool.

So that's the basic structure you will have in your db interaction method(s).

For the Login, it's a single query:

If that returns a row then the user is valid, otherwise it is not.

So, I would expect a single method on your Login class, called getUser(String name, String password), which returns a User containing whatever data you need.

Hope that's not too much of an overload!
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dave Tolls wrote:OK, looking at that Login class.

The use of that MySQL class seems to imply you are not using a connection pool, which is generally not a good idea with a webapp.  Depending on how many concurrent users you are likely to have you may find yourself running out of connections.
I'm going to treat the Login class like a DAO (Data Access Object), since that seems to be what it is.
So, when interacting with the database using JDBC in a webapp you'll generally see the following structure in a method:

You need those try-with-resources blocks to ensure the resources (the result set, statement and connection) are closed, otherwise you are going to run out.

The getConnection() call simply hides whatever mechanism you are using to get a connection...as I say above, it should be using a connection pool.

So that's the basic structure you will have in your db interaction method(s).

For the Login, it's a single query:

If that returns a row then the user is valid, otherwise it is not.

So, I would expect a single method on your Login class, called getUser(String name, String password), which returns a User containing whatever data you need.

Hope that's not too much of an overload!



Many thanks the advice, But im not usre whether mysql is the main issue here, I think the the session data isnt getting set in any of the java code, more specifically the Login bean. I know for a fact the Session Wrapper works cos i created some test servlets using my Session API and every method of the Session.class works perfectly. I'm just totally confused as it wont work here!!!
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I've created a new method in Login.class, code as follows:



Would it be better to use that? and get rid of isPssword() But wont I need isPassword() is chech if passwords match?
 
Dave Tolls
Ranch Foreman
Posts: 3009
37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It may not be the problem, but it is a problem.

At the moment that login code doesn't actually log anyone in, at least not properly.
It checks that the username exists.  It then checks that the password exists.
If those two checks pass it then grabs the user id from the database for the given username.

Assume the users table has:


If I entered:
uasername = foo
password = bam

The code as it stands would work, logging me in as foo, and  that is clearly wrong.

Fixing your Login class so it works the way pretty much every other LoginDAO works would solve that.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have these two methods in my Login.class



One is to check if the user exists and if the passwords match, if they both check out I get the userId corresponding from the DB and set it as a session var, which a few other session vars. The problem ive got is the session vars aren't being created by the Session.class. Which I know works perfectly in different scenarios! My head really is up the wall here
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
But shouldnt I be using isPassword() to check if the passwords match tho?
 
Dave Tolls
Ranch Foreman
Posts: 3009
37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No.
You user the query I gave you above.
You only want to find a user who matches both the username and password given.

So the query needs to have a WHERE clause with that AND in it.
That's how all login methods work.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dave Tolls wrote:No.
You user the query I gave you above.
You only want to find a user who matches both the username and password given.

So the query needs to have a WHERE clause with that AND in it.
That's how all login methods work.


For all these years ive been doing it incorrectly however its always worked for me lol. I'd like to do it your way. Could you give me the exact query that i'd need to use to check for both username and password? Many thanks
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ok ive redone things for the Login.class



Can everybody agree this is the correct way of doing things? Cheers.
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Michael Portman wrote:Can everybody agree this is the correct way of doing things? Cheers.


Not for a real application that's going into production for real users, no. If you're just trying to learn Java web apps then I guess it's okay, but for a real application you should use a PreparedStatement to access the database; building SQL using string concatenations allows SQL injection attacks to corrupt or damage the database. At least you're not storing plain-text passwords in the database, gotta give you credit for that.

Here's another quibble:



When you see Java code like "if (booleanValue) variable = true; else variable = false;" you can replace that code by "variable = booleanValue".
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:
Michael Portman wrote:Can everybody agree this is the correct way of doing things? Cheers.


Not for a real application that's going into production for real users, no. If you're just trying to learn Java web apps then I guess it's okay, but for a real application you should use a PreparedStatement to access the database; building SQL using string concatenations allows SQL injection attacks to corrupt or damage the database. At least you're not storing plain-text passwords in the database, gotta give you credit for that.

Here's another quibble:



When you see Java code like "if (booleanValue) variable = true; else variable = false;" you can replace that code by "variable = booleanValue".


OK ill make the change. I'm little confused over these preparedStatements are talking about. Couldn't I just escape all of my string which go into  DB with replaceAll()/replace(), or maybe even use Regex?
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ok, ive done all of the of the modifications and I still cant login. Let me give you ALL the code involved...

index.java [Servlet]



FormController.java [Servlet]



Session.java API/Wrapper [bean/Java File]



Login.java [bean/Java File]



They're all the Java files responsible for logging in the user. If you take note on line 70 it reDirects back to the index servlet with a true GET var. When I try to login I get with the var appended onto the index servlet once ive been redriected. So login is returning true with no session data is being set, thats where the problem is!! Any ideas?
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
My Session.java code definitely works cos ive just tested ALL of it's methods and not one of them dont work in the numerous test servlets ive done..
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Michael Portman wrote:I'm little confused over these preparedStatements are talking about. Couldn't I just escape all of my string which go into  DB with replaceAll()/replace(), or maybe even use Regex?


Here's the tutorial: Using Prepared Statements

As for doing something yourself to avoid SQL injection attacks: if you were a better programmer and experienced in the art of SQL injection attacks, that might be a possibility. But why go to all that work when you can just use a PreparedStatement?

(And as for escaping things, that's one of the tasks of PreparedStatement. You haven't written the code to escape quotes in the user name, anyway. But that code would be dependent on what database you were using, which is another reason to let PreparedStatement do it for you.)
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:
Michael Portman wrote:I'm little confused over these preparedStatements are talking about. Couldn't I just escape all of my string which go into  DB with replaceAll()/replace(), or maybe even use Regex?


Here's the tutorial: Using Prepared Statements

As for doing something yourself to avoid SQL injection attacks: if you were a better programmer and experienced in the art of SQL injection attacks, that might be a possibility. But why go to all that work when you can just use a PreparedStatement?

(And as for escaping things, that's one of the tasks of PreparedStatement. You haven't written the code to escape quotes in the user name, anyway. But that code would be dependent on what database you were using, which is another reason to let PreparedStatement do it for you.)


Preventing a SQL attack is the least of my worries while my site isnt live yet. Getting the user login system working is my main priority atm.
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Michael Portman wrote:Preventing a SQL attack is the least of my worries while my site isnt live yet. Getting the user login system working is my main priority atm.


"I know this is wrong, but I'll fix it later."

Problem is, there's always more important things to do later -- until the security breach happens. Really, just use a PreparedStatement. It's not like it's any harder than using a Statement, in fact it's easier because you don't have to mess with getting the single-quotes and double-quotes right.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:
Michael Portman wrote:Preventing a SQL attack is the least of my worries while my site isnt live yet. Getting the user login system working is my main priority atm.


"I know this is wrong, but I'll fix it later."

Problem is, there's always more important things to do later -- until the security breach happens. Really, just use a PreparedStatement. It's not like it's any harder than using a Statement, in fact it's easier because you don't have to mess with getting the single-quotes and double-quotes right.


Ok. I've got a MySQL.class file which handles all of my SQL queries, How can implement what are your suggestion into this:



As you can see I wouldnt know what queries went getting passed into the mysql.class if you know what I mean?
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Was it Bear who used the word "over-engineering" earlier? There's another example.

Michael Portman wrote:As you can see I wouldnt know what queries went getting passed into the mysql.class if you know what I mean?


Actually I don't know what you mean. What do you need to know that for?
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You could certainly patch up that MySQL class so that you could pass in a PreparedStatement with parameters, and then call methods to set those parameters, and then call methods to execute or update the PreparedStatement once all of the parameters are set. But there's a number of problems with the class.

First, it has static variables. This means that they are shared by all users of the MySQL class, i.e. by all servlets which use it, and it means that simultaneous requests will share those variables. This is a bad thing since you'll get infrequent errors where people are getting other people's data. But that's easily fixed by making them not be static. Then since each servlet has its own MySQL object there can't be any sharing problems. The methods shouldn't be static either.

Second, it swallows exceptions. So if something creates a MySQL object, the constructor tries to set up a database connection. If that fails for whatever reason, nothing bad happens. You just write something to the console. And then the code continues as if the MySQL object wasn't broken. It could call getResultSet(), which will return null and cause the calling code to throw NullPointerException. That's not a good thing.

Or let's suppose the connection was good and you successfully get a ResultSet from a query. So far so good. And let's suppose your second try at getting a ResultSet throws an SQL exception for some reason. Your code will log that exception and continue on, returning the ResultSet which the first query produced! This is not likely to end well either.

So, the basic rule is that if an exception is thrown which you can't handle -- that includes all of the exceptions which you're dealing with there -- then that exception should propagate all of the way up to the servlet level, where the container will deal with it. That means that the methods in your MySQL class should be throwing the exceptions, not logging them and ignoring them.

However that could lead to the calling code not being able to call the close() method, and then you have unclosed database connections floating around. Also not a good thing. I suppose you could try to deal with that in the MySQL class, so that if an exception is thrown then you try to close things before rethrowing the exception, but that would be messy. I wouldn't do that myself -- I wouldn't have this class at all, or at most I'd have a class whose only job is to get a database connection. Writing JDBC code isn't so onerous that you need to have a utility class to replace it, in my opinion.
 
Paul Clapham
Sheriff
Posts: 22708
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Michael, it may look like we're dumping a big load of complaints on your code. But let me assure you, you're just doing what a lot of people do who are new to servlets. Seems like a lot of people all run afoul of the same issues.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!