• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Servlet changes aren't taking effect

 
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hello all.

I'm trying to make changes to my Session servlet and they won't take effect. Basically I'm trying to make sure the user has confirmed their email address. My code is below...

Session.java:



The isUserConfirmed variable is always returning true, even if I hard set it to a static value of false (isUserConfirmed = false). Help much appreciated!
 
Saloon Keeper
Posts: 7582
176
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
At a quick glance, it seems that this code shouldn't even compile. The if/else in lines 70/71 is unbalanced - should there be braces around the two statements after the if condition, and also the ones in line 71?
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Tim Moores wrote:At a quick glance, it seems that this code shouldn't even compile. The if/else in lines 70/71 is unbalanced - should there be braces around the two statements after the if condition, and also the ones in line 71?



Your right I fixed it. It now compiles. I now looks like this:



However the userConfirme() method is still returning true..
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I created a test user and the confirmed column contains "no" so the value is there for the userConfirmed() method to return false..
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I've noticed something really wrong, if I change my code to this:



Is still thinks isUserConfirmed equals true, even when I hard set with a staic false value!
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Ok ive fixed it. I put the comparison into a while loop and it worked...



I don't know why I had to do that but it worked.
 
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Change the while to an if.
You only expect (at a guess) a single row back from that query.

Also, I would recommend learning to use a PreparedStatement.  This will likely involve completely restructuring how you do your database stuff.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I do use PreparedStatement, its within my MySQL class. I'll show you...



And I did change the while() to an if() and it wouldn't work. I know what you mean when I'm only getting one row but I've got to use a while() loop to get the expected results.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Sorry, you mean change this:



To this:



???
 
Tim Moores
Saloon Keeper
Posts: 7582
176
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I would also recommend not to override the service method. One should be specific about which HTTP methods are allowed, and in this case POST seems to be the only applicable one. So override the doPost method instead.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Tim Moores wrote:I would also recommend not to override the service method. One should be specific about which HTTP methods are allowed, and in this case POST seems to be the only applicable one. So override the doPost method instead.



Yeah I agree but sometimes I use both GET and POST at the same time. But in this instance I only use POST so I'll change the Session.java to doPost I suppose.
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
That's not using a PreparedStatement...well, in all but a technical way anyway.
If you can substitute a normal Statement for a PS then you aren't really using the things a PS gives you, such as protection from SQL injection, escaping Strings, and handling non-String variables correctly (eg Dates).

That query, for example, should be:

And then you can use the PreparedStatement to set the value for id.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I won't be able to implement that as the query passed in could be anything. The query could have WHERE id= or it might not. I dont really know how I would do it. All my SQL queries are executed by an external class. If you could show me an example on how to do it then ill use it.
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
You create a DAO (Data Access Object) that handles your queries.
That's the usual solution.

That DAO will have whatever methods you need, so in the above case your userConfirmed method (isUserConfirmed would be a better name) would be in this DAO.
The DAO would handle the connection, prepared statement and the handling of the result.



Something along those lines.
It should be possible to make the DAO thread safe, as you only need to keep the bits for the connection.

Edit:  Indeed, you will probably need to start looking into using a connection pool and data sources before long.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Nah I think ill just stick to what im currently using, thanks anyway. I could do a regex on the query before its passed in to prevent SQL injection. That would work wouldnt it?
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Possibly, no idea.
I wouldn't want to rely on it.
 
Sheriff
Posts: 67746
173
Mac Mac OS X IntelliJ IDE jQuery TypeScript Java iOS
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Michael Portman wrote:Nah I think ill just stick to what im currently using, thanks anyway. I could do a regex on the query before its passed in to prevent SQL injection. That would work wouldnt it?


Like wearing a sign with "Hack me!"

 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
If it's that bad then why doesn't oracle deprecate it's use?
 
Bear Bibeault
Sheriff
Posts: 67746
173
Mac Mac OS X IntelliJ IDE jQuery TypeScript Java iOS
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
When there are no malleable parameters, the overhead of a prepared statement is not needed. When there may be unsanitized input to a SQL statement, a prepared statement becomes a must.

When it comes to security, unless you are an expert that concentrates on it 100% of the time (I most certainly am not) it's best not to think that you can think of all the possible vectors of attack, and to follow the wisdom of those that have come before and have the scars to prove that security is a hard thing to get right.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
What if I just escape all my strings then? Will that prevent an injection?
 
Bear Bibeault
Sheriff
Posts: 67746
173
Mac Mac OS X IntelliJ IDE jQuery TypeScript Java iOS
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Obligatory xkcd:


Michael Portman wrote:What if I just escape all my strings then? Will that prevent an injection?


If you have to ask, that means you don't know. And that's what I'm saying about guessing about security.

Why the resistance to simply using a PreparedStatement, a known and accepted way to prevent the vast majority of SQL injection attacks?

To me, a rough analogy:

Someone 1: Airbags save lives.
Someone 2: What if I drive with a pillow in my lap?

 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Because it means rehauling my entire site on how it works with SQL! It will take forever when I've got a huge list of other tasks to do. My site isn't live yet so I could revisit this just before I go live.
 
Bear Bibeault
Sheriff
Posts: 67746
173
Mac Mac OS X IntelliJ IDE jQuery TypeScript Java iOS
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I would definitely put it on the MUST DO list prior to production deployment.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Bear Bibeault wrote:I would definitely put it on the MUST DO list prior to production deployment.



I obviously care about security and security is on the TODO list, its one of the last jobs ill be doing before we go live. My HTML designer is very secuirty conscience and he's scared about SQL attacks so it will be sorted before we go live.
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
If your website is going to be used by a reasonable number of people simultaneously then I'm not sure that this MySQL class is going to hold up:


Now, I don't know what code sits under this, but it doesn't look like it's using a connection pool.
If this is opening a new connection each time it is created then you will find this doesn't scale terribly well, or handle an inability to get a connection.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
TBH the code isn't going to be perfect on launch. That doesn't mean I won't improve it in the future, thats where v2.0, and v3.0 come in ;)
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
...and to begin with I'm not expecting a huge amount of traffic in it's early days of production. Future code revisions come with age.
 
Dave Tolls
Rancher
Posts: 4801
50
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
However, with something like this, refactoring the proper way of doing it later rather than sooner will become much harder.
If you're already baulking at the idea of changing over to connection pools and prepared statements now, it's hardly going to be easier 6 months down the line.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dave Tolls wrote:However, with something like this, refactoring the proper way of doing it later rather than sooner will become much harder.
If you're already baulking at the idea of changing over to connection pools and prepared statements now, it's hardly going to be easier 6 months down the line.



Ok I might as well look into connection pools now then. I've never worked with connection pools nor do I know how they work. Could you give me a working example of how I could implement it? Cheers buddy.
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
What server are you using?
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dave Tolls wrote:What server are you using?



Sorry I've been AFK for a while. I'm using Tomcat 7.
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
...Also should I upgrade to Tomcat 8.5? Is there much of a different between 7 and 8.5? I've been contemplating this for a while now. Is there not an edit option on this forum?
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Since you're designing this yourself I would suggest going for the higher versions, unless you can think of a good reason not to.
The newer versions are likely to be more secure, if nothing else.

For setting up a standard tomcat connection pool there's this how-to for Tomcat 8, using DBCP.
There is a newer pool for Tomcat, however you'll find more examples for DBCP (the above link).
 
Michael Portman
Ranch Hand
Posts: 129
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dave Tolls wrote:Since you're designing this yourself I would suggest going for the higher versions, unless you can think of a good reason not to.
The newer versions are likely to be more secure, if nothing else.

For setting up a standard tomcat connection pool there's this how-to for Tomcat 8, using DBCP.
There is a newer pool for Tomcat, however you'll find more examples for DBCP (the above link).



Ok thanks much appreciated! Also one final question, ill try and make it brief... Is it possible to FTP a file straight from a webpage form to a remote server? To my knowledge the only way this can be done is if the file is uploaded to the local server the traditional way, then FTP the file to the remote server, then delete the file locally. I'd much prefer if I could skip the middle bit and send the file straight from the web-form to the remote server via FTP.
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
reply
    Bookmark Topic Watch Topic
  • New Topic