Pls critique

I

Ike

I've amended a servlet I had found on the web for accessing a database. I am
wondering if you good folks could take a look at it as I am going to need it
to handle sometimes high volume simultaneously, and just want to have
someone with knowledgeable eyes take a look for perhaps glaring touble
ahead. In particular, the part below where I return the key (which is an
AUTO_INCREMENTED integer id) for any INSERT statements. This is the part
with the remarks "THIS CONERNS ME" below.

Thanks for any help. Gratefully, Ike

public class TestServlet2 extends HttpServlet {
/** Creates a new instance of TestServlet2 */
public TestServlet2() {
}
public void init(ServletConfig config) throws ServletException {
// This method initializes the servlet and only gets call once.
// Allocate all of the servlet resources here.
super.init(config);
}

public void destroy() {
// Once this method is called then any instance of this class can be
garbage collected
// Here is where all servlets resources can be deallocated.
}

// public synchronized void service (HttpServletRequest request,
HttpServletResponse response)
public void service(HttpServletRequest request, HttpServletResponse
response) throws ServletException, IOException {
try {
String returnString = "OK";;
Vector headers = new Vector();
Vector rows = new Vector();

// get an input stream from the applet
(driver,url,userid,password,query-string)
ObjectInputStream inputFromApplet = new
ObjectInputStream(request.getInputStream());
Object connArray[] = (Object[])inputFromApplet.readObject();
inputFromApplet.close();
String driver = (String)connArray[0];
String url = (String)connArray[1];
String user = (String)connArray[2];
String password = (String)connArray[3];
String SQLString = (String)connArray[4];

// perform query
int numberOfRows = 0;
Statement st = null;
Connection con = null;
try {
Class.forName(driver).newInstance();
con = DriverManager.getConnection(url,user,password);

if(SQLString.indexOf("?")>-1){//this is a prepared statement
ArrayList conntypes=((ArrayList)connArray[5]);
st =con.prepareStatement(SQLString);
for(int u=6;u<connArray.length;u++){
int t =
JavaClassNamesSQLTypes((String)conntypes.get(u-6));
switch(t){
case Types.VARCHAR:
((PreparedStatement)st).setString(u-5, (String)connArray);break;
case Types.INTEGER:
((PreparedStatement)st).setInt(u-5,
((Integer)connArray).intValue());break;
case Types.DOUBLE:
((PreparedStatement)st).setDouble(u-5,
((Double)connArray).doubleValue());break;
case Types.FLOAT:
((PreparedStatement)st).setFloat(u-5,
((Float)connArray).floatValue());break;
case Types.BOOLEAN:
((PreparedStatement)st).setBoolean(u-5,
((Boolean)connArray).booleanValue());break;
case Types.TIME:
((PreparedStatement)st).setTime(u-5, (java.sql.Time)connArray);break;
case Types.DATE:
((PreparedStatement)st).setDate(u-5, (java.sql.Date)connArray);break;
default:/*this will be the blob*/;
}
}
/* OLD WAY:
for(int u=6;u<connArray.length;u++){

if(((String)conntypes.get(u-6)).equals("java.lang.String"))
((PreparedStatement)st).setString(u-5,
(String)connArray);
}
*/
((PreparedStatement)st).execute();
}else{
st = con.createStatement();
st.execute(SQLString);
}


int updRows = st.getUpdateCount(); // -1 if no updates
if (updRows > 0) { // update,insert......
returnString = ("Rows affected: " + updRows);
}
else if (updRows == 0) { // no updates
returnString = ("Error, no rows affected");
}
else { // result of a sql-select
ResultSet rs = st.getResultSet();
ResultSetMetaData md = rs.getMetaData();
// headers
int numberOfColumns = md.getColumnCount();
for(int column = 0; column < numberOfColumns; column++)
{
headers.addElement(md.getColumnLabel(column+1));
}

// result
while (rs.next()) {
numberOfRows++;
Vector newRow = new Vector();
for (int i = 1; i <= numberOfColumns; i++) {
//newRow.addElement(rs.getString(i));
//newRow.addElement(rs.getObject(i));

if(((String)headers.get(i-1)).equals("picture")){
try{
Blob blob = rs.getBlob(i);
if(blob!=null){
int iLength = (int)(blob.length());
byte[] jack = blob.getBytes( 1,
iLength );

newRow.addElement(jack);
}else{
//dont bother - null Objects do not
get added to a Vector
newRow.addElement(new
javax.swing.ImageIcon().getImage());
}
}catch(Exception ex){returnString=
ex.toString();}
} else
newRow.addElement(rs.getObject(i));
}
// System.out.println("\n row:
"+(rs.getObject(i)));

rows.addElement(newRow);
}
rs.close();
if (numberOfRows == 0) returnString = "no rows
selected";
}
st.close();
/** THIS CONERNS
ME...........****************************************************/
if(SQLString.startsWith("INSERT")){//if it is an insert,
return the id (which is an INT) as String
ResultSet rs=null;
try{
st = con.createStatement();
rs = st.executeQuery("SELECT LAST_INSERT_ID()");
if (rs.next())
returnString=Integer.toString(rs.getInt(1));
}catch(Exception ignore0){}
if(rs!=null)
rs.close();
if(st!=null)
st.close();
}
/***************************************************************************
/
con.close();
}
catch (SQLException e) {
if (st != null) st.close();
if (con != null) con.close();
returnString = e.toString();
}
// send objects back to applet
ObjectOutputStream outputToApplet = new
ObjectOutputStream(response.getOutputStream());
outputToApplet.writeObject(returnString); // sql-message
outputToApplet.writeObject(headers); // fieldnames
outputToApplet.writeObject(rows); // result-vector
outputToApplet.flush();
outputToApplet.close();
}
catch(Exception e) {
e.printStackTrace();
}
}

public static int JavaClassNamesSQLTypes(String sqltype){
if(sqltype.equals("java.lang.Boolean"))
//return Types.BIT;
return Types.BOOLEAN;
else if(sqltype.equals("java.math.BigInteger"))
return Types.BIGINT;
else if(sqltype.equals("java.math.BigDecimal"))
return Types.DECIMAL;
else if(sqltype.equals("java.lang.String"))
return Types.VARCHAR;
else if(sqltype.equals("java.lang.Integer"))
return Types.INTEGER;
else if(sqltype.equals("java.lang.Short"))
return Types.SMALLINT;
else if(sqltype.equals("java.lang.Byte"))
return Types.TINYINT;
else if(sqltype.equals("java.lang.Double"))
return Types.DOUBLE;
else if(sqltype.equals("java.lang.Float"))
return Types.FLOAT;
else if(sqltype.equals("java.sql.Time"))
return Types.TIME;
else if(sqltype.equals("java.sql.Timestamp"))
return Types.TIMESTAMP;
else if(sqltype.equals("java.util.Date"))
return Types.DATE;
else
return Types.BLOB;
}
}
 
J

John C. Bollinger

Ike said:
I've amended a servlet I had found on the web for accessing a database. I am
wondering if you good folks could take a look at it as I am going to need it
to handle sometimes high volume simultaneously, and just want to have
someone with knowledgeable eyes take a look for perhaps glaring touble
ahead. In particular, the part below where I return the key (which is an
AUTO_INCREMENTED integer id) for any INSERT statements. This is the part
with the remarks "THIS CONERNS ME" below.

Thanks for any help. Gratefully, Ike
Click to expand...

Ike, I think you'd have been better off starting from scratch. There is
pretty much nothing that I like about the servlet. Some specific
problems:

(1) the service(HttpServletRequest, HttpServletResponse) method is
overridden instead of one or more of the method-specific methods (doGet,
doPost, etc.)
(2) methods init(ServletConfig) and destroy() are needlessly overidden
(3) Passing serialized objects back and forth between client and servlet
is probably unwise unless necessary. It is more usual to put the the
necessary parameters into request parameters of either a GET or a PUT
request. Special handling might be necessary if long data needed to be
supported, but that's not the norm. If you need to return structured
data in the response (rather than an HTML page for human eyes) then
consider formatting it in XML -- that opens it to a wider range of clients.
(4) the servlet provides for a client-specified driver to be loaded
[this is a big security hole]
(5) the servlet provides for a client-specified DB URL [another big
security hole]
(6) the servlet allows the client to supply arbitrary SQL statements [an
enormous, gaping security hole!] This servlet "avoids" SQL injection
attacks by redefining them as the normal mode of operation.
(7) Despite the fact that the servlet allows the client to do just about
anything he wants to do, it is not in fact DBMS-neutral because it
specifies particular mappings from Java types to SQL types that may or
may not be best or even appropriate for the DBMS actually contacted.
(And see also below.)
(8) There is special handling for a column name "picture" that seems
specific to a particular DB schema. This sort of handling is okay for a
much more narrowly focused servlet, but oughtn't to run through the
result set metadata in any case.
(9) The part that concerns you, should. It is not thread-safe;
moreover, it depends on a DB function that is not present in some DBMSs.
In fact, there was no standard way to perform that operation
(obtaining auto-generated keys) until Java 1.4, and JDBC drivers may or
may not support the new JDBC feature to do so.

There are likely other problems. It looks like what you started with
was demonstration code, not intended for production use.

A more typical approach is for the servlet to provide an abstraction
layer between the DB and the clients. The servlet is generally
configured to talk to a specific DB using a specific driver (although
these may be part of the servlet's or webapp's configuration) and to
execute only well-controlled statements based on the requests made by
clients. Often authentication to the DB is quite separate from
authentication to the webapp -- the client authenticates to the webapp,
but only the webapp authenticates to the DB.

You are also likely get much better performance if you use a connection
pool instead of opening a new connection with each request. You may
also benefit from using PreparedStatements; note, however, that these
are not generally thread-safe, so the way to go would probably be to
pool them along with the connections. (I.e. pool some sort of DB-access
objects instead of bare Connections.)

I really don't see anything salvageable here. Sorry.


John Bollinger
(e-mail address removed)

public class TestServlet2 extends HttpServlet {
/** Creates a new instance of TestServlet2 */
public TestServlet2() {
}
public void init(ServletConfig config) throws ServletException {
// This method initializes the servlet and only gets call once.
// Allocate all of the servlet resources here.
super.init(config);
}

public void destroy() {
// Once this method is called then any instance of this class can be
garbage collected
// Here is where all servlets resources can be deallocated.
}

// public synchronized void service (HttpServletRequest request,
HttpServletResponse response)
public void service(HttpServletRequest request, HttpServletResponse
response) throws ServletException, IOException {
try {
String returnString = "OK";;
Vector headers = new Vector();
Vector rows = new Vector();

// get an input stream from the applet
(driver,url,userid,password,query-string)
ObjectInputStream inputFromApplet = new
ObjectInputStream(request.getInputStream());
Object connArray[] = (Object[])inputFromApplet.readObject();
inputFromApplet.close();
String driver = (String)connArray[0];
String url = (String)connArray[1];
String user = (String)connArray[2];
String password = (String)connArray[3];
String SQLString = (String)connArray[4];

// perform query
int numberOfRows = 0;
Statement st = null;
Connection con = null;
try {
Class.forName(driver).newInstance();
con = DriverManager.getConnection(url,user,password);

if(SQLString.indexOf("?")>-1){//this is a prepared statement
ArrayList conntypes=((ArrayList)connArray[5]);
st =con.prepareStatement(SQLString);
for(int u=6;u<connArray.length;u++){
int t =
JavaClassNamesSQLTypes((String)conntypes.get(u-6));
switch(t){
case Types.VARCHAR:
((PreparedStatement)st).setString(u-5, (String)connArray);break;
case Types.INTEGER:
((PreparedStatement)st).setInt(u-5,
((Integer)connArray).intValue());break;
case Types.DOUBLE:
((PreparedStatement)st).setDouble(u-5,
((Double)connArray).doubleValue());break;
case Types.FLOAT:
((PreparedStatement)st).setFloat(u-5,
((Float)connArray).floatValue());break;
case Types.BOOLEAN:
((PreparedStatement)st).setBoolean(u-5,
((Boolean)connArray).booleanValue());break;
case Types.TIME:
((PreparedStatement)st).setTime(u-5, (java.sql.Time)connArray);break;
case Types.DATE:
((PreparedStatement)st).setDate(u-5, (java.sql.Date)connArray);break;
default:/*this will be the blob*/;
}
}
/* OLD WAY:
for(int u=6;u<connArray.length;u++){

if(((String)conntypes.get(u-6)).equals("java.lang.String"))
((PreparedStatement)st).setString(u-5,
(String)connArray);
}
*/
((PreparedStatement)st).execute();
}else{
st = con.createStatement();
st.execute(SQLString);
}


int updRows = st.getUpdateCount(); // -1 if no updates
if (updRows > 0) { // update,insert......
returnString = ("Rows affected: " + updRows);
}
else if (updRows == 0) { // no updates
returnString = ("Error, no rows affected");
}
else { // result of a sql-select
ResultSet rs = st.getResultSet();
ResultSetMetaData md = rs.getMetaData();
// headers
int numberOfColumns = md.getColumnCount();
for(int column = 0; column < numberOfColumns; column++)
{
headers.addElement(md.getColumnLabel(column+1));
}

// result
while (rs.next()) {
numberOfRows++;
Vector newRow = new Vector();
for (int i = 1; i <= numberOfColumns; i++) {
//newRow.addElement(rs.getString(i));
//newRow.addElement(rs.getObject(i));

if(((String)headers.get(i-1)).equals("picture")){
try{
Blob blob = rs.getBlob(i);
if(blob!=null){
int iLength = (int)(blob.length());
byte[] jack = blob.getBytes( 1,
iLength );

newRow.addElement(jack);
}else{
//dont bother - null Objects do not
get added to a Vector
newRow.addElement(new
javax.swing.ImageIcon().getImage());
}
}catch(Exception ex){returnString=
ex.toString();}
} else
newRow.addElement(rs.getObject(i));
}
// System.out.println("\n row:
"+(rs.getObject(i)));

rows.addElement(newRow);
}
rs.close();
if (numberOfRows == 0) returnString = "no rows
selected";
}
st.close();
/** THIS CONERNS
ME...........****************************************************/
if(SQLString.startsWith("INSERT")){//if it is an insert,
return the id (which is an INT) as String
ResultSet rs=null;
try{
st = con.createStatement();
rs = st.executeQuery("SELECT LAST_INSERT_ID()");
if (rs.next())
returnString=Integer.toString(rs.getInt(1));
}catch(Exception ignore0){}
if(rs!=null)
rs.close();
if(st!=null)
st.close();
}
/***************************************************************************
/
con.close();
}
catch (SQLException e) {
if (st != null) st.close();
if (con != null) con.close();
returnString = e.toString();
}
// send objects back to applet
ObjectOutputStream outputToApplet = new
ObjectOutputStream(response.getOutputStream());
outputToApplet.writeObject(returnString); // sql-message
outputToApplet.writeObject(headers); // fieldnames
outputToApplet.writeObject(rows); // result-vector
outputToApplet.flush();
outputToApplet.close();
}
catch(Exception e) {
e.printStackTrace();
}
}

public static int JavaClassNamesSQLTypes(String sqltype){
if(sqltype.equals("java.lang.Boolean"))
//return Types.BIT;
return Types.BOOLEAN;
else if(sqltype.equals("java.math.BigInteger"))
return Types.BIGINT;
else if(sqltype.equals("java.math.BigDecimal"))
return Types.DECIMAL;
else if(sqltype.equals("java.lang.String"))
return Types.VARCHAR;
else if(sqltype.equals("java.lang.Integer"))
return Types.INTEGER;
else if(sqltype.equals("java.lang.Short"))
return Types.SMALLINT;
else if(sqltype.equals("java.lang.Byte"))
return Types.TINYINT;
else if(sqltype.equals("java.lang.Double"))
return Types.DOUBLE;
else if(sqltype.equals("java.lang.Float"))
return Types.FLOAT;
else if(sqltype.equals("java.sql.Time"))
return Types.TIME;
else if(sqltype.equals("java.sql.Timestamp"))
return Types.TIMESTAMP;
else if(sqltype.equals("java.util.Date"))
return Types.DATE;
else
return Types.BLOB;
}
}
Click to expand...
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

ObjectInput/Output streams and byte[] 1
Slow compared to php 7
Blobs via servlet 0
Lexical Analysis on C++ 1
Connected SQLite to my java program but information are not submitted 2
Could someone help me and explain what is wrong here? 4
School Project 1
Dont work, it´s something whit the loops? 1

Members online

Total: 31 (members: 2, guests: 29)
Robots: 270

Forum statistics

Threads
473,755
Messages
2,569,534
Members
45,007
Latest member
obedient dusk

Latest Threads

Top