From: Lane S. <la...@op...> - 2006-02-20 00:57:48
|
<html><body><font style=3D"font-family: arial,helvetica,sans-serif;" size= =3D"2">Hi Nikolaos.<br> <br> This is most disturbing to hear about.<br> <br> First, there is a known concurrency issue with WM in 2.0 and possibly in prior releases. If you look at the mail thread dating back for 45 days, you will definitely see it.<br> <br> My concern is that the webmacro instance, shared, and the context, not shared, is improperly orchestrated for concurrency. The webmacro instance is local to WMServlet. You might consider synchronizing access to this variable as a trial fix in your local calls below. This will introduce some serialization of web processing but it might help to verify the solution.<br> <br> One thing you might consider trying is to use a different cache handler. A few years ago, I wrote one and it is a part of the distro. I have never had a problem using this cache handler.<br> <br> Lane<br><br>--- Nikolaos Papadakis <nk...@cc...> wrote:<br><= br>From: Nikolaos Papadakis <nk...@cc...><br>Date: Sat, 18 F= eb 2006 10:50:33 +0200<br>To: web...@li...<br>Subjec= t: [WebMacro-user] [WebMacro-user<br><br>Hi all, <br> <br>I am using webmac= ro to develop a web-based application. <br>Things seemed to work fine till = yesterday. A customer complained that he <br>managed to =E2=80=9Csee=E2=80= =9D the account of another customer when he logged-in using <br>his own cr= edentials (he send me a screen shot of the =E2=80=9Cview=E2=80=9D). <br>By = inspecting the log file I found that both users have been logged-in <br>at= the same time (the second customer logged in 3 sec after the first <br>cu= stomer). <br>The system has more than 1000 customers but this inconvenience= happened <br>for the first time yesterday (?). <br>It seems that differen= t sessions have been mixed up. <br>The servlet includes a handle method and= uses several other methods <br>(methods that write and read from a databa= se). <br>All methods are defined in the same class (that extends WMServlet)= . <br>As far as I know servlets must not have instance variables (only loca= l <br>variables -inside methods- to prevent data corruption and <br>incon= sistencies). However in my servlet I use only one non local <br>variable (= a Logger) and I don't think this is the cause of the problem. <br>As an att= ached file I send you a fragment of the before mentioned servlet. <br>Can i= nstance variables be defined in the case of a servlet that extends <br>WMS= ervlet? (I mean is it safe? or its not a good practice without <br>keeping= precautions?) <br>Do you have any idea what can have caused this inconveni= ence ? <br>Is the use of methods inside the servlet thread-safe ? <br> <br>= Please help!!! <br>Nikolaos Papadakis <br></font></body></html> |
From: Keats K. <ke...@xa...> - 2006-02-20 03:34:27
|
It's not possible to tell from the code snippet what is going wrong. The concurrency issue that Lane is referring to is fairly obscure (only affecting the #include directive with dynamic template names if I recall correctly) and not relevant here. The code as shown looks OK, but we can't see how the USERID is stored in the session, or how the user data is stored and retrieved based on this USERID. The bug could be in the data access layer or the session management subsystem (which depends on your Servlet container), or in some caching mechanism. It is unlikely, but not impossible that this could be a WebMacro bug. If I had to guess, I would suspect that the code which actually retrieves the user data is not properly synchronized. Send us a bit more information about your application, and maybe we can help you track this down. Keats Lane Sharman wrote: > Hi Nikolaos. > > This is most disturbing to hear about. > > First, there is a known concurrency issue with WM in 2.0 and possibly > in prior releases. If you look at the mail thread dating back for 45 > days, you will definitely see it. > > My concern is that the webmacro instance, shared, and the context, not > shared, is improperly orchestrated for concurrency. The webmacro > instance is local to WMServlet. You might consider synchronizing > access to this variable as a trial fix in your local calls below. This > will introduce some serialization of web processing but it might help > to verify the solution. > > One thing you might consider trying is to use a different cache > handler. A few years ago, I wrote one and it is a part of the distro. > I have never had a problem using this cache handler. > > Lane > > --- Nikolaos Papadakis <nk...@cc...> wrote: > > From: Nikolaos Papadakis <nk...@cc...> > Date: Sat, 18 Feb 2006 10:50:33 +0200 > To: web...@li... > Subject: [WebMacro-user] [WebMacro-user > > Hi all, > > I am using webmacro to develop a web-based application. > Things seemed to work fine till yesterday. A customer complained that he > managed to “see” the account of another customer when he logged-in using > his own credentials (he send me a screen shot of the “view”). > By inspecting the log file I found that both users have been logged-in > at the same time (the second customer logged in 3 sec after the first > customer). > The system has more than 1000 customers but this inconvenience happened > for the first time yesterday (?). > It seems that different sessions have been mixed up. > The servlet includes a handle method and uses several other methods > (methods that write and read from a database). > All methods are defined in the same class (that extends WMServlet). > As far as I know servlets must not have instance variables (only local > variables -inside methods- to prevent data corruption and > inconsistencies). However in my servlet I use only one non local > variable (a Logger) and I don't think this is the cause of the problem. > As an attached file I send you a fragment of the before mentioned > servlet. > Can instance variables be defined in the case of a servlet that extends > WMServlet? (I mean is it safe? or its not a good practice without > keeping precautions?) > Do you have any idea what can have caused this inconvenience ? > Is the use of methods inside the servlet thread-safe ? > > Please help!!! > Nikolaos Papadakis > ------------------------------------------------------- This SF.net > email is sponsored by: Splunk Inc. Do you grep through log files for > problems? Stop! Download the new AJAX search engine that makes > searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! > http://sel.as-us.falkag.net/sel?cmd=k&kid3432&bid#0486&dat1642 > _______________________________________________ Webmacro-user mailing > list Web...@li... > https://lists.sourceforge.net/lists/listinfo/webmacro-user |
From: Nikolaos P. <nk...@cc...> - 2006-02-20 19:04:17
Attachments:
Login.java
|
Hi all, I am sending a cut (but indicative---I hope) version of the core servlet=20 I am using. The userId variable is set into the session by the validateUser method. Various other methods have been omitted in this version for readability=20 reasons. Hope it is clear. Thank you in advance Nikolaos Papadakis P.S. Just for the record the configuration I am using includes: SuSe ver 10.0 Java 1.4.2 Tomcat 5.0.30 in conjunction with Apache ver 2.0.53. I also use some customized version of S2J to =E2=80=9Cobjectize=E2=80=9D = tables and=20 relations in the db. Keats Kirsch wrote: > It's not possible to tell from the code snippet what is going wrong. =20 > The concurrency issue that Lane is referring to is fairly obscure=20 > (only affecting the #include directive with dynamic template names if=20 > I recall correctly) and not relevant here. > > The code as shown looks OK, but we can't see how the USERID is stored=20 > in the session, or how the user data is stored and retrieved based on=20 > this USERID. The bug could be in the data access layer or the session=20 > management subsystem (which depends on your Servlet container), or in=20 > some caching mechanism. It is unlikely, but not impossible that this=20 > could be a WebMacro bug. If I had to guess, I would suspect that the=20 > code which actually retrieves the user data is not properly synchronize= d. > > Send us a bit more information about your application, and maybe we=20 > can help you track this down. > > Keats > > Lane Sharman wrote: > >> Hi Nikolaos. >> >> This is most disturbing to hear about. >> >> First, there is a known concurrency issue with WM in 2.0 and possibly=20 >> in prior releases. If you look at the mail thread dating back for 45=20 >> days, you will definitely see it. >> >> My concern is that the webmacro instance, shared, and the context,=20 >> not shared, is improperly orchestrated for concurrency. The webmacro=20 >> instance is local to WMServlet. You might consider synchronizing=20 >> access to this variable as a trial fix in your local calls below.=20 >> This will introduce some serialization of web processing but it might=20 >> help to verify the solution. >> >> One thing you might consider trying is to use a different cache=20 >> handler. A few years ago, I wrote one and it is a part of the distro.=20 >> I have never had a problem using this cache handler. >> >> Lane >> >> --- Nikolaos Papadakis <nk...@cc...> wrote: >> >> From: Nikolaos Papadakis <nk...@cc...> >> Date: Sat, 18 Feb 2006 10:50:33 +0200 >> To: web...@li... >> Subject: [WebMacro-user] [WebMacro-user >> >> Hi all, >> >> I am using webmacro to develop a web-based application. >> Things seemed to work fine till yesterday. A customer complained that = he >> managed to =E2=80=9Csee=E2=80=9D the account of another customer when = he logged-in using >> his own credentials (he send me a screen shot of the =E2=80=9Cview=E2=80= =9D). >> By inspecting the log file I found that both users have been logged-in >> at the same time (the second customer logged in 3 sec after the first >> customer). >> The system has more than 1000 customers but this inconvenience happene= d >> for the first time yesterday (?). >> It seems that different sessions have been mixed up. >> The servlet includes a handle method and uses several other methods >> (methods that write and read from a database). >> All methods are defined in the same class (that extends WMServlet). >> As far as I know servlets must not have instance variables (only local >> variables -inside methods- to prevent data corruption and >> inconsistencies). However in my servlet I use only one non local >> variable (a Logger) and I don't think this is the cause of the problem. >> As an attached file I send you a fragment of the before mentioned=20 >> servlet. >> Can instance variables be defined in the case of a servlet that extend= s >> WMServlet? (I mean is it safe? or its not a good practice without >> keeping precautions?) >> Do you have any idea what can have caused this inconvenience ? >> Is the use of methods inside the servlet thread-safe ? >> >> Please help!!! >> Nikolaos Papadakis >> ------------------------------------------------------- This SF.net=20 >> email is sponsored by: Splunk Inc. Do you grep through log files for=20 >> problems? Stop! Download the new AJAX search engine that makes=20 >> searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!=20 >> http://sel.as-us.falkag.net/sel?cmd=3Dk&kid=103432&bid#0486&dat=121642= =20 >> _______________________________________________ Webmacro-user mailing=20 >> list Web...@li...=20 >> https://lists.sourceforge.net/lists/listinfo/webmacro-user=20 > > > > > > ------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. Do you grep through log=20 > files > for problems? Stop! Download the new AJAX search engine that makes > searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! > http://sel.as-us.falkag.net/sel?cmd=3Dlnk&kid=3D103432&bid=3D230486&dat= =3D121642 > _______________________________________________ > Webmacro-user mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/webmacro-user > > |
From: Keats K. <ke...@xa...> - 2006-02-21 03:28:59
|
Nikolaos, I don't know S2J, but I assume it's an O/R mapping tool. I would focus my attention on the following line of code: Users[] foundUsers = UsersManager.getManager().loadByWhere( "where USE_EMAIL=\'" + login + "\' AND USE_PASSWORD=\'" + password + "\'", connection); Is this invoking customized code? Is loadByWhere() synchronized, or is there some other synchronization mechanism here? (Btw, unless the S2J protects against it, this code could be vulnerable to a SQL-injection attack. Make sure you check the userid and password for apostrophes or other special characters that might trip up your RDBMS.) Hope this helps. Keats Nikolaos Papadakis wrote: > Hi all, > I am sending a cut (but indicative---I hope) version of the core > servlet I am using. > The userId variable is set into the session by the validateUser method. > Various other methods have been omitted in this version for > readability reasons. > Hope it is clear. > > Thank you in advance > Nikolaos Papadakis > > P.S. Just for the record the configuration I am using includes: > SuSe ver 10.0 > Java 1.4.2 > Tomcat 5.0.30 in conjunction with Apache ver 2.0.53. > I also use some customized version of S2J to “objectize” tables and > relations in the db. > > > > > > Keats Kirsch wrote: > >> It's not possible to tell from the code snippet what is going wrong. >> The concurrency issue that Lane is referring to is fairly obscure >> (only affecting the #include directive with dynamic template names if >> I recall correctly) and not relevant here. >> >> The code as shown looks OK, but we can't see how the USERID is stored >> in the session, or how the user data is stored and retrieved based on >> this USERID. The bug could be in the data access layer or the >> session management subsystem (which depends on your Servlet >> container), or in some caching mechanism. It is unlikely, but not >> impossible that this could be a WebMacro bug. If I had to guess, I >> would suspect that the code which actually retrieves the user data is >> not properly synchronized. >> >> Send us a bit more information about your application, and maybe we >> can help you track this down. >> >> Keats >> >> Lane Sharman wrote: >> >>> Hi Nikolaos. >>> >>> This is most disturbing to hear about. >>> >>> First, there is a known concurrency issue with WM in 2.0 and >>> possibly in prior releases. If you look at the mail thread dating >>> back for 45 days, you will definitely see it. >>> >>> My concern is that the webmacro instance, shared, and the context, >>> not shared, is improperly orchestrated for concurrency. The webmacro >>> instance is local to WMServlet. You might consider synchronizing >>> access to this variable as a trial fix in your local calls below. >>> This will introduce some serialization of web processing but it >>> might help to verify the solution. >>> >>> One thing you might consider trying is to use a different cache >>> handler. A few years ago, I wrote one and it is a part of the >>> distro. I have never had a problem using this cache handler. >>> >>> Lane >>> >>> --- Nikolaos Papadakis <nk...@cc...> wrote: >>> >>> From: Nikolaos Papadakis <nk...@cc...> >>> Date: Sat, 18 Feb 2006 10:50:33 +0200 >>> To: web...@li... >>> Subject: [WebMacro-user] [WebMacro-user >>> >>> Hi all, >>> >>> I am using webmacro to develop a web-based application. >>> Things seemed to work fine till yesterday. A customer complained >>> that he >>> managed to “see” the account of another customer when he logged-in >>> using >>> his own credentials (he send me a screen shot of the “view”). >>> By inspecting the log file I found that both users have been logged-in >>> at the same time (the second customer logged in 3 sec after the first >>> customer). >>> The system has more than 1000 customers but this inconvenience happened >>> for the first time yesterday (?). >>> It seems that different sessions have been mixed up. >>> The servlet includes a handle method and uses several other methods >>> (methods that write and read from a database). >>> All methods are defined in the same class (that extends WMServlet). >>> As far as I know servlets must not have instance variables (only local >>> variables -inside methods- to prevent data corruption and >>> inconsistencies). However in my servlet I use only one non local >>> variable (a Logger) and I don't think this is the cause of the problem. >>> As an attached file I send you a fragment of the before mentioned >>> servlet. >>> Can instance variables be defined in the case of a servlet that extends >>> WMServlet? (I mean is it safe? or its not a good practice without >>> keeping precautions?) >>> Do you have any idea what can have caused this inconvenience ? >>> Is the use of methods inside the servlet thread-safe ? >>> >>> Please help!!! >>> Nikolaos Papadakis >>> ------------------------------------------------------- This SF.net >>> email is sponsored by: Splunk Inc. Do you grep through log files for >>> problems? Stop! Download the new AJAX search engine that makes >>> searching your log files as easy as surfing the web. DOWNLOAD >>> SPLUNK! >>> http://sel.as-us.falkag.net/sel?cmd=k&kid3432&bid#0486&dat1642 >>> _______________________________________________ Webmacro-user >>> mailing list Web...@li... >>> https://lists.sourceforge.net/lists/listinfo/webmacro-user >> >> >> >> >> >> >> ------------------------------------------------------- >> This SF.net email is sponsored by: Splunk Inc. Do you grep through >> log files >> for problems? Stop! Download the new AJAX search engine that makes >> searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! >> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642 >> _______________________________________________ >> Webmacro-user mailing list >> Web...@li... >> https://lists.sourceforge.net/lists/listinfo/webmacro-user >> >> >------------------------------------------------------------------------ > >package org.nkpap.visualizer; > >import java.io.BufferedOutputStream; >import java.io.ByteArrayOutputStream; >import java.io.File; >import java.io.FileInputStream; >import java.io.FileNotFoundException; >import java.io.IOException; >import java.io.InputStream; >import java.io.OutputStream; >import java.io.PrintStream; >import java.sql.Connection; >import java.sql.DriverManager; >import java.sql.PreparedStatement; >import java.sql.ResultSet; >import java.sql.SQLException; >import java.text.DateFormat; >import java.text.SimpleDateFormat; >import java.util.Calendar; >import java.util.Date; >import java.util.Enumeration; >import java.util.Hashtable; >import java.util.LinkedList; >import java.util.List; >import java.util.logging.FileHandler; >import java.util.logging.Logger; > >import javax.servlet.ServletException; >import javax.servlet.http.HttpServletRequest; >import javax.servlet.http.HttpServletResponse; >import javax.servlet.http.HttpSession; > >import org.netoperis.db.core.Authors; >import org.netoperis.db.core.Interests; >import org.netoperis.db.core.Reviews; >import org.netoperis.db.core.Submissions; >import org.netoperis.db.core.TopicList; >import org.netoperis.db.core.Topics; >import org.netoperis.db.core.Users; >import org.netoperis.db.core.Votes; >import org.netoperis.db.mgr.AuthorsManager; >import org.netoperis.db.mgr.InterestsManager; >import org.netoperis.db.mgr.ReviewsManager; >import org.netoperis.db.mgr.SubmissionsManager; >import org.netoperis.db.mgr.TopicListManager; >import org.netoperis.db.mgr.TopicsManager; >import org.netoperis.db.mgr.UsersManager; >import org.netoperis.db.mgr.VotesManager; >import org.nkpap.util.EmailBuffer; >import org.nkpap.util.MyConnectionManager; >import org.nkpap.util.RandomPasswordGenerator; >import org.webmacro.PropertyException; >import org.webmacro.ResourceException; >import org.webmacro.Template; >import org.webmacro.servlet.HandlerException; >import org.webmacro.servlet.WMServlet; >import org.webmacro.servlet.WebContext; >import org.webmacro.util.Settings; > >import com.oreilly.servlet.MultipartRequest; > >/** > * @author nkpap in Greece 2 ��� 2005 > * > * (c) 2005 - 2006 > * > */ >public class Login extends WMServlet { > public static Settings configProperties = null; > Logger logger; > > protected void start() throws ServletException { > try { > configProperties = new Settings(); > System.out.println(" READING CONFIGURATION FILE: ms2.properties"); > > configProperties.load("ms2.properties"); > System.out > .println(" CONFIGURATION FILE: ms2.properties LOADED SUCCESSFULLY "); > FileHandler handler = new FileHandler(configProperties > .getSetting("LogsPath") > + "my_log.txt", true); > logger = Logger.getLogger("org.nkpap.visualizer.LoginUsers"); > logger.addHandler(handler); > } catch (Exception ex) { > System.out > .println("ERROR READING CONFIGURATION FILE: ms2.properties"); > System.out.println(ex); > } > } > > private Template returnMyTemplate(String template, WebContext context) { > try { > context.put("fragment", template); > return getTemplate("users/index.htm"); > } catch (ResourceException e) { > System.out > .println("Oooops!!! a problem occured with the error template!"); > e.printStackTrace(); > return null; > } > } > > public Template handle(WebContext context) throws HandlerException { > HttpServletRequest request = context.getRequest(); > HttpSession session = request.getSession(); > context.getResponse().setContentType("text/html; charset=iso-8859-7"); > > String userID = (String) session.getAttribute("USERID"); > > if (isActionEqualTo("login", context)) { > // use has entered credentials > String login = request.getParameter("login"); > String password = request.getParameter("password"); > > if (validateUser(login, password, context)) { > if (session.getAttribute("FIRSTIME") != null) > return showReviewer( > (String) session.getAttribute("USERID"), context); > return returnMyTemplate("users/choices.htm", context); > }// user doesnt validate > session.invalidate(); > context.put("fail", "Access denied !!"); > return returnMyTemplate("users/login.htm", context); > } > // -----------------------ACTIONS---BEGIN----HERE------------- > // ACTIONS that do not require user to be logged-in > if (isActionEqualTo("register", context)) { > return returnMyTemplate("users/aut_registration.htm", context); > } > if (isActionEqualTo("back", context)) { > return returnMyTemplate("users/login.htm", context); > } > if (isActionEqualTo("remind", context)) { > return returnMyTemplate("users/remind_passwd.htm", context); > } > // end of ACTIONS that do not require user to be logged-in > > if (isActionEqualTo("logout", context)) { > session.invalidate(); > logger.info("User " + userID + " logged out"); > return returnMyTemplate("users/login.htm", context); > } > // --------------------ACTIONS----END-----HERE--------- > // the default page to be shown... > > if (request.getParameter("action") == null && userID != null) { > if (session.getAttribute("FIRSTIME") != null) > return showReviewer((String) session.getAttribute("USERID"), > context); > return returnMyTemplate("users/choices.htm", context); > } else { > return returnMyTemplate("users/login.htm", context); > } > } > > private Template handleError(Exception e, Connection connection, > WebContext context) { > logger.severe("An error occurred:\n" + e.toString()); > if (connection != null) { > try { > connection.close(); > } catch (SQLException e1) { > e1.printStackTrace(); > } > } > e.printStackTrace(); > context.put("msg", "An error occurred:\n" + e.toString()); > return returnMyTemplate("users/errore.htm", context); > } > > private Template showReviewer(String userId, WebContext context) { > MyConnectionManager ConnectionManager = new MyConnectionManager(); > Connection connection = null; > logger.info("starting method: showReviewer"); > try { > int usId = Integer.parseInt(userId); > connection = ConnectionManager.getConnection(); > > Users user = UsersManager.getManager().loadByKey(usId, connection); > Topics[] topics = TopicsManager.getManager().loadAll(connection); > > connection.close(); > context.put("user", user); > // context.put("interests", interests); > context.put("topics", topics); > } catch (SQLException e) { > return handleError(e, connection, context); > } > > logger.info("ended method: showReviewer"); > return returnMyTemplate("users/rev_registration.htm", context); > } > > private boolean isActionEqualTo(String actionName, WebContext context) { > HttpServletRequest request = context.getRequest(); > if (request.getParameter("action") != null > && request.getParameter("action").equalsIgnoreCase(actionName)) > return true; > return false; > } > > private Template showJobs(WebContext context) { > Connection connection = null; > MyConnectionManager ConnectionManager = new MyConnectionManager(); > HttpServletRequest request = context.getRequest(); > HttpSession session = request.getSession(); > String userId = (String) session.getAttribute("USERID"); > logger.info("starting method: showJobs by user: " + userId); > > try { > connection = ConnectionManager.getConnection(); > Reviews[] reviews = ReviewsManager.getManager().loadByWhere( > "where USE_ID=\'" + userId + "\'", connection); > // System.out.println("Papers : " + submissions.length); > > for (int i = 0; i < reviews.length; i++) { > > Submissions subm = SubmissionsManager.getManager().loadByKey( > reviews[i].getSubId(), connection); > > reviews[i].setForeignTitle(subm.getSubPaperTitle()); > reviews[i].setForeignStatus(subm.getSubStatus()); > } > connection.close(); > context.put("jobs", reviews); > logger.info("ended method: showJobs by user: " + userId); > return returnMyTemplate("users/rev_home.htm", context); > } catch (SQLException e) { > return handleError(e, connection, context); > } > } > > private boolean validateUser(String login, String password, > WebContext context) { > MyConnectionManager ConnectionManager = new MyConnectionManager(); > HttpServletRequest request = context.getRequest(); > HttpSession session = request.getSession(); > Connection connection = null; > logger.info("starting method: validateUser login: " + login > + " passwd: " + password); > boolean userOK = false; > try { > connection = ConnectionManager.getConnection(); > Users[] foundUsers = UsersManager.getManager().loadByWhere( > "where USE_EMAIL=\'" + login + "\' AND USE_PASSWORD=\'" > + password + "\'", connection); > if (foundUsers.length != 0) { > userOK = true; > if (foundUsers[0].getUseActive() == 0) { > session.setAttribute("FIRSTIME", "TRUE"); > } > session.setAttribute("USERID", foundUsers[0] > .getUseId()+""); > } > connection.close(); > } catch (SQLException e) { > handleError(e, connection, context); > return false; > } > > logger.info("ended method: validateUser: " + userOK + " login: " > + login + " passwd: " + password); > return userOK; > } >} > |
From: Nikolaos P. <nk...@cc...> - 2006-02-21 06:52:33
|
Dear Keats, thank you for replying. The code is protected against SQL-injections attacks. Special characters=20 and apostrophes are manipulated properly to avoid potential attackers=20 execute a different sql query. The classes I use are based on S2J (an O/R mapping tool) with a lot of=20 new additions. Now concerning the snippet you mentioned, from the servlet developer's=20 prospective, each client is another thread that calls the servlet via=20 the service(), doPost(), or doGet() methods (while only one instance=20 exists). Every thread will carry its specific userId (through its session given=20 by the servlet container). Every thread will execute the same =E2=80=9Cwhere USE_EMAIL.....=E2=80=9D= snippet but=20 with different parameters (email address and password in this case). So I think there is no need the loadByWhere() method to be synchronized. The only synchronized mechanism, however, is used in the part of code=20 that is responsible for inserting a new column into the db (because I am=20 not using the autoincrement feature of the specific RDBMS), but that's=20 not pertinent here. Please correct me if I've gotten it wrong. Best Regards, Nikos Papadakis Keats Kirsch wrote: > Nikolaos, > > I don't know S2J, but I assume it's an O/R mapping tool. I would=20 > focus my attention on the following line of code: > > Users[] foundUsers =3D UsersManager.getManager().loadByWhere= ( > "where USE_EMAIL=3D\'" + login + "\' AND=20 > USE_PASSWORD=3D\'" > + password + "\'", connection); > > Is this invoking customized code? Is loadByWhere() synchronized, or=20 > is there some other synchronization mechanism here? > > (Btw, unless the S2J protects against it, this code could be=20 > vulnerable to a SQL-injection attack. Make sure you check the userid=20 > and password for apostrophes or other special characters that might=20 > trip up your RDBMS.) > > Hope this helps. > > Keats > > Nikolaos Papadakis wrote: > >> Hi all, > >> I am sending a cut (but indicative---I hope) version of the core=20 >> servlet I am using. >> The userId variable is set into the session by the validateUser method. >> Various other methods have been omitted in this version for=20 >> readability reasons. >> Hope it is clear. >> >> Thank you in advance >> Nikolaos Papadakis >> >> P.S. Just for the record the configuration I am using includes: >> SuSe ver 10.0 >> Java 1.4.2 >> Tomcat 5.0.30 in conjunction with Apache ver 2.0.53. >> I also use some customized version of S2J to =E2=80=9Cobjectize=E2=80=9D= tables and=20 >> relations in the db. >> >> >> >> >> >> Keats Kirsch wrote: >> >>> It's not possible to tell from the code snippet what is going=20 >>> wrong. The concurrency issue that Lane is referring to is fairly=20 >>> obscure (only affecting the #include directive with dynamic template=20 >>> names if I recall correctly) and not relevant here. >>> >>> The code as shown looks OK, but we can't see how the USERID is=20 >>> stored in the session, or how the user data is stored and retrieved=20 >>> based on this USERID. The bug could be in the data access layer or=20 >>> the session management subsystem (which depends on your Servlet=20 >>> container), or in some caching mechanism. It is unlikely, but not=20 >>> impossible that this could be a WebMacro bug. If I had to guess, I=20 >>> would suspect that the code which actually retrieves the user data=20 >>> is not properly synchronized. >>> >>> Send us a bit more information about your application, and maybe we=20 >>> can help you track this down. >>> >>> Keats >>> >>> Lane Sharman wrote: >>> >>>> Hi Nikolaos. >>>> >>>> This is most disturbing to hear about. >>>> >>>> First, there is a known concurrency issue with WM in 2.0 and=20 >>>> possibly in prior releases. If you look at the mail thread dating=20 >>>> back for 45 days, you will definitely see it. >>>> >>>> My concern is that the webmacro instance, shared, and the context,=20 >>>> not shared, is improperly orchestrated for concurrency. The=20 >>>> webmacro instance is local to WMServlet. You might consider=20 >>>> synchronizing access to this variable as a trial fix in your local=20 >>>> calls below. This will introduce some serialization of web=20 >>>> processing but it might help to verify the solution. >>>> >>>> One thing you might consider trying is to use a different cache=20 >>>> handler. A few years ago, I wrote one and it is a part of the=20 >>>> distro. I have never had a problem using this cache handler. >>>> >>>> Lane >>>> >>>> --- Nikolaos Papadakis <nk...@cc...> wrote: >>>> >>>> From: Nikolaos Papadakis <nk...@cc...> >>>> Date: Sat, 18 Feb 2006 10:50:33 +0200 >>>> To: web...@li... >>>> Subject: [WebMacro-user] [WebMacro-user >>>> >>>> Hi all, >>>> >>>> I am using webmacro to develop a web-based application. >>>> Things seemed to work fine till yesterday. A customer complained=20 >>>> that he >>>> managed to =E2=80=9Csee=E2=80=9D the account of another customer whe= n he logged-in=20 >>>> using >>>> his own credentials (he send me a screen shot of the =E2=80=9Cview=E2= =80=9D). >>>> By inspecting the log file I found that both users have been logged-= in >>>> at the same time (the second customer logged in 3 sec after the firs= t >>>> customer). >>>> The system has more than 1000 customers but this inconvenience=20 >>>> happened >>>> for the first time yesterday (?). >>>> It seems that different sessions have been mixed up. >>>> The servlet includes a handle method and uses several other methods >>>> (methods that write and read from a database). >>>> All methods are defined in the same class (that extends WMServlet). >>>> As far as I know servlets must not have instance variables (only loc= al >>>> variables -inside methods- to prevent data corruption and >>>> inconsistencies). However in my servlet I use only one non local >>>> variable (a Logger) and I don't think this is the cause of the=20 >>>> problem. >>>> As an attached file I send you a fragment of the before mentioned=20 >>>> servlet. >>>> Can instance variables be defined in the case of a servlet that=20 >>>> extends >>>> WMServlet? (I mean is it safe? or its not a good practice without >>>> keeping precautions?) >>>> Do you have any idea what can have caused this inconvenience ? >>>> Is the use of methods inside the servlet thread-safe ? >>>> >>>> Please help!!! >>>> Nikolaos Papadakis >>>> ------------------------------------------------------- This SF.net=20 >>>> email is sponsored by: Splunk Inc. Do you grep through log files=20 >>>> for problems? Stop! Download the new AJAX search engine that makes=20 >>>> searching your log files as easy as surfing the web. DOWNLOAD=20 >>>> SPLUNK!=20 >>>> http://sel.as-us.falkag.net/sel?cmd=3Dk&kid=103432&bid#0486&dat=1216= 42=20 >>>> _______________________________________________ Webmacro-user=20 >>>> mailing list Web...@li...=20 >>>> https://lists.sourceforge.net/lists/listinfo/webmacro-user=20 >>> >>> >>> >>> >>> >>> >>> ------------------------------------------------------- >>> This SF.net email is sponsored by: Splunk Inc. Do you grep through=20 >>> log files >>> for problems? Stop! Download the new AJAX search engine that makes >>> searching your log files as easy as surfing the web. DOWNLOAD SPLUN= K! >>> http://sel.as-us.falkag.net/sel?cmd=3Dlnk&kid=3D103432&bid=3D230486&d= at=3D121642=20 >>> >>> _______________________________________________ >>> Webmacro-user mailing list >>> Web...@li... >>> https://lists.sourceforge.net/lists/listinfo/webmacro-user >>> >>> >> ----------------------------------------------------------------------= -- >> >> package org.nkpap.visualizer; >> >> import java.io.BufferedOutputStream; >> import java.io.ByteArrayOutputStream; >> import java.io.File; >> import java.io.FileInputStream; >> import java.io.FileNotFoundException; >> import java.io.IOException; >> import java.io.InputStream; >> import java.io.OutputStream; >> import java.io.PrintStream; >> import java.sql.Connection; >> import java.sql.DriverManager; >> import java.sql.PreparedStatement; >> import java.sql.ResultSet; >> import java.sql.SQLException; >> import java.text.DateFormat; >> import java.text.SimpleDateFormat; >> import java.util.Calendar; >> import java.util.Date; >> import java.util.Enumeration; >> import java.util.Hashtable; >> import java.util.LinkedList; >> import java.util.List; >> import java.util.logging.FileHandler; >> import java.util.logging.Logger; >> >> import javax.servlet.ServletException; >> import javax.servlet.http.HttpServletRequest; >> import javax.servlet.http.HttpServletResponse; >> import javax.servlet.http.HttpSession; >> >> import org.netoperis.db.core.Authors; >> import org.netoperis.db.core.Interests; >> import org.netoperis.db.core.Reviews; >> import org.netoperis.db.core.Submissions; >> import org.netoperis.db.core.TopicList; >> import org.netoperis.db.core.Topics; >> import org.netoperis.db.core.Users; >> import org.netoperis.db.core.Votes; >> import org.netoperis.db.mgr.AuthorsManager; >> import org.netoperis.db.mgr.InterestsManager; >> import org.netoperis.db.mgr.ReviewsManager; >> import org.netoperis.db.mgr.SubmissionsManager; >> import org.netoperis.db.mgr.TopicListManager; >> import org.netoperis.db.mgr.TopicsManager; >> import org.netoperis.db.mgr.UsersManager; >> import org.netoperis.db.mgr.VotesManager; >> import org.nkpap.util.EmailBuffer; >> import org.nkpap.util.MyConnectionManager; >> import org.nkpap.util.RandomPasswordGenerator; >> import org.webmacro.PropertyException; >> import org.webmacro.ResourceException; >> import org.webmacro.Template; >> import org.webmacro.servlet.HandlerException; >> import org.webmacro.servlet.WMServlet; >> import org.webmacro.servlet.WebContext; >> import org.webmacro.util.Settings; >> >> import com.oreilly.servlet.MultipartRequest; >> >> /** >> * @author nkpap in Greece 2 =EF=BF=BD=EF=BF=BD=EF=BF=BD 2005 >> * * (c) 2005 - 2006 >> * */ >> public class Login extends WMServlet { >> public static Settings configProperties =3D null; >> Logger logger; >> >> protected void start() throws ServletException { >> try { >> configProperties =3D new Settings(); >> System.out.println(" READING CONFIGURATION FILE:=20 >> ms2.properties"); >> >> configProperties.load("ms2.properties"); >> System.out >> .println(" CONFIGURATION FILE: ms2.properties=20 >> LOADED SUCCESSFULLY "); >> FileHandler handler =3D new FileHandler(configProperties >> .getSetting("LogsPath") >> + "my_log.txt", true); >> logger =3D=20 >> Logger.getLogger("org.nkpap.visualizer.LoginUsers"); >> logger.addHandler(handler); >> } catch (Exception ex) { >> System.out >> .println("ERROR READING CONFIGURATION FILE:=20 >> ms2.properties"); >> System.out.println(ex); >> } >> } >> >> private Template returnMyTemplate(String template, WebContext=20 >> context) { >> try { >> context.put("fragment", template); >> return getTemplate("users/index.htm"); >> } catch (ResourceException e) { >> System.out >> .println("Oooops!!! a problem occured with the=20 >> error template!"); >> e.printStackTrace(); >> return null; >> } >> } >> >> public Template handle(WebContext context) throws HandlerException= { >> HttpServletRequest request =3D context.getRequest(); >> HttpSession session =3D request.getSession(); >> context.getResponse().setContentType("text/html;=20 >> charset=3Diso-8859-7"); >> >> String userID =3D (String) session.getAttribute("USERID"); >> >> if (isActionEqualTo("login", context)) { >> // use has entered credentials >> String login =3D request.getParameter("login"); >> String password =3D request.getParameter("password"); >> >> if (validateUser(login, password, context)) { >> if (session.getAttribute("FIRSTIME") !=3D null) >> return showReviewer( >> (String) session.getAttribute("USERID"),=20 >> context); >> return returnMyTemplate("users/choices.htm", context); >> }// user doesnt validate >> session.invalidate(); >> context.put("fail", "Access denied !!"); >> return returnMyTemplate("users/login.htm", context); >> } >> // -----------------------ACTIONS---BEGIN----HERE------------- >> // ACTIONS that do not require user to be logged-in >> if (isActionEqualTo("register", context)) { >> return returnMyTemplate("users/aut_registration.htm",=20 >> context); >> } >> if (isActionEqualTo("back", context)) { >> return returnMyTemplate("users/login.htm", context); >> } >> if (isActionEqualTo("remind", context)) { >> return returnMyTemplate("users/remind_passwd.htm", context= ); >> } >> // end of ACTIONS that do not require user to be logged-in >> >> if (isActionEqualTo("logout", context)) { >> session.invalidate(); >> logger.info("User " + userID + " logged out"); >> return returnMyTemplate("users/login.htm", context); >> } =20 >> // --------------------ACTIONS----END-----HERE--------- >> // the default page to be shown... >> >> if (request.getParameter("action") =3D=3D null && userID !=3D = null) { >> if (session.getAttribute("FIRSTIME") !=3D null) >> return showReviewer((String)=20 >> session.getAttribute("USERID"), >> context); >> return returnMyTemplate("users/choices.htm", context); >> } else { >> return returnMyTemplate("users/login.htm", context); >> } >> } >> >> private Template handleError(Exception e, Connection connection, >> WebContext context) { >> logger.severe("An error occurred:\n" + e.toString()); >> if (connection !=3D null) { >> try { >> connection.close(); >> } catch (SQLException e1) { >> e1.printStackTrace(); >> } >> } >> e.printStackTrace(); >> context.put("msg", "An error occurred:\n" + e.toString()); >> return returnMyTemplate("users/errore.htm", context); >> } >> >> private Template showReviewer(String userId, WebContext context) { >> MyConnectionManager ConnectionManager =3D new=20 >> MyConnectionManager(); >> Connection connection =3D null; >> logger.info("starting method: showReviewer"); >> try { >> int usId =3D Integer.parseInt(userId); >> connection =3D ConnectionManager.getConnection(); >> >> Users user =3D UsersManager.getManager().loadByKey(usId,=20 >> connection); >> Topics[] topics =3D=20 >> TopicsManager.getManager().loadAll(connection); >> >> connection.close(); >> context.put("user", user); >> // context.put("interests", interests); >> context.put("topics", topics); >> } catch (SQLException e) { >> return handleError(e, connection, context); >> } >> >> logger.info("ended method: showReviewer"); >> return returnMyTemplate("users/rev_registration.htm", context)= ; >> } >> >> private boolean isActionEqualTo(String actionName, WebContext=20 >> context) { >> HttpServletRequest request =3D context.getRequest(); >> if (request.getParameter("action") !=3D null >> &&=20 >> request.getParameter("action").equalsIgnoreCase(actionName)) >> return true; >> return false; >> } >> >> private Template showJobs(WebContext context) { >> Connection connection =3D null; >> MyConnectionManager ConnectionManager =3D new=20 >> MyConnectionManager(); >> HttpServletRequest request =3D context.getRequest(); >> HttpSession session =3D request.getSession(); >> String userId =3D (String) session.getAttribute("USERID"); >> logger.info("starting method: showJobs by user: " + userId); >> >> try { >> connection =3D ConnectionManager.getConnection(); >> Reviews[] reviews =3D ReviewsManager.getManager().loadByWh= ere( >> "where USE_ID=3D\'" + userId + "\'", connection); >> // System.out.println("Papers : " + submissions.length); >> >> for (int i =3D 0; i < reviews.length; i++) { >> >> Submissions subm =3D=20 >> SubmissionsManager.getManager().loadByKey( >> reviews[i].getSubId(), connection); >> >> reviews[i].setForeignTitle(subm.getSubPaperTitle()); >> reviews[i].setForeignStatus(subm.getSubStatus()); >> } >> connection.close(); >> context.put("jobs", reviews); >> logger.info("ended method: showJobs by user: " + userId); >> return returnMyTemplate("users/rev_home.htm", context); >> } catch (SQLException e) { >> return handleError(e, connection, context); >> } >> } >> >> private boolean validateUser(String login, String password, >> WebContext context) { >> MyConnectionManager ConnectionManager =3D new=20 >> MyConnectionManager(); >> HttpServletRequest request =3D context.getRequest(); >> HttpSession session =3D request.getSession(); >> Connection connection =3D null; >> logger.info("starting method: validateUser login: " + login >> + " passwd: " + password); >> boolean userOK =3D false; >> try { >> connection =3D ConnectionManager.getConnection(); >> Users[] foundUsers =3D UsersManager.getManager().loadByWhe= re( >> "where USE_EMAIL=3D\'" + login + "\' AND=20 >> USE_PASSWORD=3D\'" >> + password + "\'", connection); >> if (foundUsers.length !=3D 0) { >> userOK =3D true; >> if (foundUsers[0].getUseActive() =3D=3D 0) { >> session.setAttribute("FIRSTIME", "TRUE"); >> } >> session.setAttribute("USERID", foundUsers[0] >> .getUseId()+""); >> } >> connection.close(); >> } catch (SQLException e) { >> handleError(e, connection, context); >> return false; >> } >> >> logger.info("ended method: validateUser: " + userOK + " login:= " >> + login + " passwd: " + password); >> return userOK; >> } >> } >> > > > > > ------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. Do you grep through log=20 > files > for problems? Stop! Download the new AJAX search engine that makes > searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! > http://sel.as-us.falkag.net/sel?cmd=3Dlnk&kid=3D103432&bid=3D230486&dat= =3D121642 > _______________________________________________ > Webmacro-user mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/webmacro-user > > |
From: Keats K. <ke...@xa...> - 2006-02-21 15:43:39
|
Nikolaos, If UsersManager.getManager() can return a shared Manager instance, which I presume it does, then its loadByWhere() needs synchronization. Since the method is allocating the Users array, a second concurrent request could trash the value of the first request. You need to synchronize the method (which would implicitly synchronize on the Manager instance), or synchronize on some other shared object. Concurrency is a tricky business. I'll put in a plug for WM developer Brian Goetz's book on the subject. Unfortunately it's not coming out until June, but you can pre-order it on Amazon. In the meantime, check out Doug Lea's writings. Keats Nikolaos Papadakis wrote: > Dear Keats, > > thank you for replying. > The code is protected against SQL-injections attacks. Special > characters and apostrophes are manipulated properly to avoid potential > attackers execute a different sql query. > The classes I use are based on S2J (an O/R mapping tool) with a lot of > new additions. > > Now concerning the snippet you mentioned, from the servlet developer's > prospective, each client is another thread that calls the servlet via > the service(), doPost(), or doGet() methods (while only one instance > exists). > Every thread will carry its specific userId (through its session given > by the servlet container). > Every thread will execute the same “where USE_EMAIL.....” snippet but > with different parameters (email address and password in this case). > So I think there is no need the loadByWhere() method to be synchronized. > The only synchronized mechanism, however, is used in the part of code > that is responsible for inserting a new column into the db (because I > am not using the autoincrement feature of the specific RDBMS), but > that's not pertinent here. > > Please correct me if I've gotten it wrong. > > > > Best Regards, > Nikos Papadakis > > > Keats Kirsch wrote: > >> Nikolaos, >> >> I don't know S2J, but I assume it's an O/R mapping tool. I would >> focus my attention on the following line of code: >> >> Users[] foundUsers = UsersManager.getManager().loadByWhere( >> "where USE_EMAIL=\'" + login + "\' AND >> USE_PASSWORD=\'" >> + password + "\'", connection); >> >> Is this invoking customized code? Is loadByWhere() synchronized, or >> is there some other synchronization mechanism here? >> >> (Btw, unless the S2J protects against it, this code could be >> vulnerable to a SQL-injection attack. Make sure you check the userid >> and password for apostrophes or other special characters that might >> trip up your RDBMS.) >> >> Hope this helps. >> >> Keats >> >> Nikolaos Papadakis wrote: >> >>> Hi all, >> >> >>> I am sending a cut (but indicative---I hope) version of the core >>> servlet I am using. >>> The userId variable is set into the session by the validateUser method. >>> Various other methods have been omitted in this version for >>> readability reasons. >>> Hope it is clear. >>> >>> Thank you in advance >>> Nikolaos Papadakis >>> >>> P.S. Just for the record the configuration I am using includes: >>> SuSe ver 10.0 >>> Java 1.4.2 >>> Tomcat 5.0.30 in conjunction with Apache ver 2.0.53. >>> I also use some customized version of S2J to “objectize” tables and >>> relations in the db. >>> >>> >>> >>> >>> >>> Keats Kirsch wrote: >>> >>>> It's not possible to tell from the code snippet what is going >>>> wrong. The concurrency issue that Lane is referring to is fairly >>>> obscure (only affecting the #include directive with dynamic >>>> template names if I recall correctly) and not relevant here. >>>> >>>> The code as shown looks OK, but we can't see how the USERID is >>>> stored in the session, or how the user data is stored and retrieved >>>> based on this USERID. The bug could be in the data access layer or >>>> the session management subsystem (which depends on your Servlet >>>> container), or in some caching mechanism. It is unlikely, but not >>>> impossible that this could be a WebMacro bug. If I had to guess, I >>>> would suspect that the code which actually retrieves the user data >>>> is not properly synchronized. >>>> >>>> Send us a bit more information about your application, and maybe we >>>> can help you track this down. >>>> >>>> Keats >>>> >>>> Lane Sharman wrote: >>>> >>>>> Hi Nikolaos. >>>>> >>>>> This is most disturbing to hear about. >>>>> >>>>> First, there is a known concurrency issue with WM in 2.0 and >>>>> possibly in prior releases. If you look at the mail thread dating >>>>> back for 45 days, you will definitely see it. >>>>> >>>>> My concern is that the webmacro instance, shared, and the context, >>>>> not shared, is improperly orchestrated for concurrency. The >>>>> webmacro instance is local to WMServlet. You might consider >>>>> synchronizing access to this variable as a trial fix in your local >>>>> calls below. This will introduce some serialization of web >>>>> processing but it might help to verify the solution. >>>>> >>>>> One thing you might consider trying is to use a different cache >>>>> handler. A few years ago, I wrote one and it is a part of the >>>>> distro. I have never had a problem using this cache handler. >>>>> >>>>> Lane >>>>> >>>>> --- Nikolaos Papadakis <nk...@cc...> wrote: >>>>> >>>>> From: Nikolaos Papadakis <nk...@cc...> >>>>> Date: Sat, 18 Feb 2006 10:50:33 +0200 >>>>> To: web...@li... >>>>> Subject: [WebMacro-user] [WebMacro-user >>>>> >>>>> Hi all, >>>>> >>>>> I am using webmacro to develop a web-based application. >>>>> Things seemed to work fine till yesterday. A customer complained >>>>> that he >>>>> managed to “see” the account of another customer when he logged-in >>>>> using >>>>> his own credentials (he send me a screen shot of the “view”). >>>>> By inspecting the log file I found that both users have been >>>>> logged-in >>>>> at the same time (the second customer logged in 3 sec after the first >>>>> customer). >>>>> The system has more than 1000 customers but this inconvenience >>>>> happened >>>>> for the first time yesterday (?). >>>>> It seems that different sessions have been mixed up. >>>>> The servlet includes a handle method and uses several other methods >>>>> (methods that write and read from a database). >>>>> All methods are defined in the same class (that extends WMServlet). >>>>> As far as I know servlets must not have instance variables (only >>>>> local >>>>> variables -inside methods- to prevent data corruption and >>>>> inconsistencies). However in my servlet I use only one non local >>>>> variable (a Logger) and I don't think this is the cause of the >>>>> problem. >>>>> As an attached file I send you a fragment of the before mentioned >>>>> servlet. >>>>> Can instance variables be defined in the case of a servlet that >>>>> extends >>>>> WMServlet? (I mean is it safe? or its not a good practice without >>>>> keeping precautions?) >>>>> Do you have any idea what can have caused this inconvenience ? >>>>> Is the use of methods inside the servlet thread-safe ? >>>>> >>>>> Please help!!! >>>>> Nikolaos Papadakis >>>>> ------------------------------------------------------- This >>>>> SF.net email is sponsored by: Splunk Inc. Do you grep through log >>>>> files for problems? Stop! Download the new AJAX search engine that >>>>> makes searching your log files as easy as surfing the web. >>>>> DOWNLOAD SPLUNK! >>>>> http://sel.as-us.falkag.net/sel?cmd=k&kid3432&bid#0486&dat1642 >>>>> _______________________________________________ Webmacro-user >>>>> mailing list Web...@li... >>>>> https://lists.sourceforge.net/lists/listinfo/webmacro-user >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> ------------------------------------------------------- >>>> This SF.net email is sponsored by: Splunk Inc. Do you grep through >>>> log files >>>> for problems? Stop! Download the new AJAX search engine that makes >>>> searching your log files as easy as surfing the web. DOWNLOAD >>>> SPLUNK! >>>> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642 >>>> >>>> _______________________________________________ >>>> Webmacro-user mailing list >>>> Web...@li... >>>> https://lists.sourceforge.net/lists/listinfo/webmacro-user >>>> >>>> >>> ------------------------------------------------------------------------ >>> >>> >>> package org.nkpap.visualizer; >>> >>> import java.io.BufferedOutputStream; >>> import java.io.ByteArrayOutputStream; >>> import java.io.File; >>> import java.io.FileInputStream; >>> import java.io.FileNotFoundException; >>> import java.io.IOException; >>> import java.io.InputStream; >>> import java.io.OutputStream; >>> import java.io.PrintStream; >>> import java.sql.Connection; >>> import java.sql.DriverManager; >>> import java.sql.PreparedStatement; >>> import java.sql.ResultSet; >>> import java.sql.SQLException; >>> import java.text.DateFormat; >>> import java.text.SimpleDateFormat; >>> import java.util.Calendar; >>> import java.util.Date; >>> import java.util.Enumeration; >>> import java.util.Hashtable; >>> import java.util.LinkedList; >>> import java.util.List; >>> import java.util.logging.FileHandler; >>> import java.util.logging.Logger; >>> >>> import javax.servlet.ServletException; >>> import javax.servlet.http.HttpServletRequest; >>> import javax.servlet.http.HttpServletResponse; >>> import javax.servlet.http.HttpSession; >>> >>> import org.netoperis.db.core.Authors; >>> import org.netoperis.db.core.Interests; >>> import org.netoperis.db.core.Reviews; >>> import org.netoperis.db.core.Submissions; >>> import org.netoperis.db.core.TopicList; >>> import org.netoperis.db.core.Topics; >>> import org.netoperis.db.core.Users; >>> import org.netoperis.db.core.Votes; >>> import org.netoperis.db.mgr.AuthorsManager; >>> import org.netoperis.db.mgr.InterestsManager; >>> import org.netoperis.db.mgr.ReviewsManager; >>> import org.netoperis.db.mgr.SubmissionsManager; >>> import org.netoperis.db.mgr.TopicListManager; >>> import org.netoperis.db.mgr.TopicsManager; >>> import org.netoperis.db.mgr.UsersManager; >>> import org.netoperis.db.mgr.VotesManager; >>> import org.nkpap.util.EmailBuffer; >>> import org.nkpap.util.MyConnectionManager; >>> import org.nkpap.util.RandomPasswordGenerator; >>> import org.webmacro.PropertyException; >>> import org.webmacro.ResourceException; >>> import org.webmacro.Template; >>> import org.webmacro.servlet.HandlerException; >>> import org.webmacro.servlet.WMServlet; >>> import org.webmacro.servlet.WebContext; >>> import org.webmacro.util.Settings; >>> >>> import com.oreilly.servlet.MultipartRequest; >>> >>> /** >>> * @author nkpap in Greece 2 ��� 2005 >>> * * (c) 2005 - 2006 >>> * */ >>> public class Login extends WMServlet { >>> public static Settings configProperties = null; >>> Logger logger; >>> >>> protected void start() throws ServletException { >>> try { >>> configProperties = new Settings(); >>> System.out.println(" READING CONFIGURATION FILE: >>> ms2.properties"); >>> >>> configProperties.load("ms2.properties"); >>> System.out >>> .println(" CONFIGURATION FILE: ms2.properties >>> LOADED SUCCESSFULLY "); >>> FileHandler handler = new FileHandler(configProperties >>> .getSetting("LogsPath") >>> + "my_log.txt", true); >>> logger = >>> Logger.getLogger("org.nkpap.visualizer.LoginUsers"); >>> logger.addHandler(handler); >>> } catch (Exception ex) { >>> System.out >>> .println("ERROR READING CONFIGURATION FILE: >>> ms2.properties"); >>> System.out.println(ex); >>> } >>> } >>> >>> private Template returnMyTemplate(String template, WebContext >>> context) { >>> try { >>> context.put("fragment", template); >>> return getTemplate("users/index.htm"); >>> } catch (ResourceException e) { >>> System.out >>> .println("Oooops!!! a problem occured with the >>> error template!"); >>> e.printStackTrace(); >>> return null; >>> } >>> } >>> >>> public Template handle(WebContext context) throws >>> HandlerException { >>> HttpServletRequest request = context.getRequest(); >>> HttpSession session = request.getSession(); >>> context.getResponse().setContentType("text/html; >>> charset=iso-8859-7"); >>> >>> String userID = (String) session.getAttribute("USERID"); >>> >>> if (isActionEqualTo("login", context)) { >>> // use has entered credentials >>> String login = request.getParameter("login"); >>> String password = request.getParameter("password"); >>> >>> if (validateUser(login, password, context)) { >>> if (session.getAttribute("FIRSTIME") != null) >>> return showReviewer( >>> (String) session.getAttribute("USERID"), >>> context); >>> return returnMyTemplate("users/choices.htm", context); >>> }// user doesnt validate >>> session.invalidate(); >>> context.put("fail", "Access denied !!"); >>> return returnMyTemplate("users/login.htm", context); >>> } >>> // -----------------------ACTIONS---BEGIN----HERE------------- >>> // ACTIONS that do not require user to be logged-in >>> if (isActionEqualTo("register", context)) { >>> return returnMyTemplate("users/aut_registration.htm", >>> context); >>> } >>> if (isActionEqualTo("back", context)) { >>> return returnMyTemplate("users/login.htm", context); >>> } >>> if (isActionEqualTo("remind", context)) { >>> return returnMyTemplate("users/remind_passwd.htm", >>> context); >>> } >>> // end of ACTIONS that do not require user to be logged-in >>> >>> if (isActionEqualTo("logout", context)) { >>> session.invalidate(); >>> logger.info("User " + userID + " logged out"); >>> return returnMyTemplate("users/login.htm", context); >>> } // >>> --------------------ACTIONS----END-----HERE--------- >>> // the default page to be shown... >>> >>> if (request.getParameter("action") == null && userID != null) { >>> if (session.getAttribute("FIRSTIME") != null) >>> return showReviewer((String) >>> session.getAttribute("USERID"), >>> context); >>> return returnMyTemplate("users/choices.htm", context); >>> } else { >>> return returnMyTemplate("users/login.htm", context); >>> } >>> } >>> >>> private Template handleError(Exception e, Connection connection, >>> WebContext context) { >>> logger.severe("An error occurred:\n" + e.toString()); >>> if (connection != null) { >>> try { >>> connection.close(); >>> } catch (SQLException e1) { >>> e1.printStackTrace(); >>> } >>> } >>> e.printStackTrace(); >>> context.put("msg", "An error occurred:\n" + e.toString()); >>> return returnMyTemplate("users/errore.htm", context); >>> } >>> >>> private Template showReviewer(String userId, WebContext context) { >>> MyConnectionManager ConnectionManager = new >>> MyConnectionManager(); >>> Connection connection = null; >>> logger.info("starting method: showReviewer"); >>> try { >>> int usId = Integer.parseInt(userId); >>> connection = ConnectionManager.getConnection(); >>> >>> Users user = UsersManager.getManager().loadByKey(usId, >>> connection); >>> Topics[] topics = >>> TopicsManager.getManager().loadAll(connection); >>> >>> connection.close(); >>> context.put("user", user); >>> // context.put("interests", interests); >>> context.put("topics", topics); >>> } catch (SQLException e) { >>> return handleError(e, connection, context); >>> } >>> >>> logger.info("ended method: showReviewer"); >>> return returnMyTemplate("users/rev_registration.htm", context); >>> } >>> >>> private boolean isActionEqualTo(String actionName, WebContext >>> context) { >>> HttpServletRequest request = context.getRequest(); >>> if (request.getParameter("action") != null >>> && >>> request.getParameter("action").equalsIgnoreCase(actionName)) >>> return true; >>> return false; >>> } >>> >>> private Template showJobs(WebContext context) { >>> Connection connection = null; >>> MyConnectionManager ConnectionManager = new >>> MyConnectionManager(); >>> HttpServletRequest request = context.getRequest(); >>> HttpSession session = request.getSession(); >>> String userId = (String) session.getAttribute("USERID"); >>> logger.info("starting method: showJobs by user: " + userId); >>> >>> try { >>> connection = ConnectionManager.getConnection(); >>> Reviews[] reviews = >>> ReviewsManager.getManager().loadByWhere( >>> "where USE_ID=\'" + userId + "\'", connection); >>> // System.out.println("Papers : " + submissions.length); >>> >>> for (int i = 0; i < reviews.length; i++) { >>> >>> Submissions subm = >>> SubmissionsManager.getManager().loadByKey( >>> reviews[i].getSubId(), connection); >>> >>> reviews[i].setForeignTitle(subm.getSubPaperTitle()); >>> reviews[i].setForeignStatus(subm.getSubStatus()); >>> } >>> connection.close(); >>> context.put("jobs", reviews); >>> logger.info("ended method: showJobs by user: " + userId); >>> return returnMyTemplate("users/rev_home.htm", context); >>> } catch (SQLException e) { >>> return handleError(e, connection, context); >>> } >>> } >>> >>> private boolean validateUser(String login, String password, >>> WebContext context) { >>> MyConnectionManager ConnectionManager = new >>> MyConnectionManager(); >>> HttpServletRequest request = context.getRequest(); >>> HttpSession session = request.getSession(); >>> Connection connection = null; >>> logger.info("starting method: validateUser login: " + login >>> + " passwd: " + password); >>> boolean userOK = false; >>> try { >>> connection = ConnectionManager.getConnection(); >>> Users[] foundUsers = UsersManager.getManager().loadByWhere( >>> "where USE_EMAIL=\'" + login + "\' AND >>> USE_PASSWORD=\'" >>> + password + "\'", connection); >>> if (foundUsers.length != 0) { >>> userOK = true; >>> if (foundUsers[0].getUseActive() == 0) { >>> session.setAttribute("FIRSTIME", "TRUE"); >>> } >>> session.setAttribute("USERID", foundUsers[0] >>> .getUseId()+""); >>> } >>> connection.close(); >>> } catch (SQLException e) { >>> handleError(e, connection, context); >>> return false; >>> } >>> >>> logger.info("ended method: validateUser: " + userOK + " >>> login: " >>> + login + " passwd: " + password); >>> return userOK; >>> } >>> } >>> >> >> >> >> |
From: Marc P. <ma...@an...> - 2006-02-20 10:31:27
|
On 20 Feb 2006, at 00:57, Lane Sharman wrote: > > This is most disturbing to hear about. > > First, there is a known concurrency issue with WM in 2.0 and possibly > in prior releases. If you look at the mail thread dating back for 45 > days, you will definitely see it. > > My concern is that the webmacro instance, shared, and the context, not > shared, is improperly orchestrated for concurrency. The webmacro > instance is local to WMServlet. You might consider synchronizing > access to this variable as a trial fix in your local calls below. > This will introduce some serialization of web processing but it > might help to verify the solution. I am slightly alarmed by this also, because I have seen something similar happening. On a recent commercial website I did the back-end for, using Groovy and WebMacro, I have seen on occasion clients submitting data to the site using somebody else's UID. I'm am not 100% sure there is a definite problem because the times I have seen it was during intense activity by a lot of users at the same company, so it may have been people sharing each others' PCs (it was a promotional game playing event). Anyway, I will be investigating this further in the next few days and will let you know what I find out. Actually my fear is that the problem is in Groovy rather than WebMacro (WM being much easier to fix!) because WM is not really used to retrieve the session UID except in some niche scenarios. The more work I do the more I realise that sessions are evil and problematic... ditch them and pass around a request ID with every link is the best solution - especially as it supports multiple concurrent workflows/requests. Cheers |
From: Nikolaos P. <nk...@cc...> - 2006-02-20 19:03:13
|
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <meta content=3D"text/html;charset=3DUTF-8" http-equiv=3D"Content-Type"= > <title></title> </head> <body bgcolor=3D"#ffffff" text=3D"#000000"> Hi Lane,<br> <br> Lane Sharman wrote: <blockquote cite=3D"mid...@dm..." type=3D"cite"><font style=3D"font-family: arial,helvetica,sans-serif;" size=3D"2">Hi Nikolaos.<br> <br> This is most disturbing to hear about.<br> </font></blockquote> <br> Thank you for replying.<br> <blockquote cite=3D"mid...@dm..." type=3D"cite"><font style=3D"font-family: arial,helvetica,sans-serif;" size=3D"2"><br> First, there is a known concurrency issue with WM in 2.0 and possibly in prior releases. If you look at the mail thread dating back for 45 days, you will definitely see it.<br> </font></blockquote> <br> I will take a look at that thread.<br> <br> <blockquote cite=3D"mid...@dm..." type=3D"cite"><font style=3D"font-family: arial,helvetica,sans-serif;" size=3D"2"><br> My concern is that the webmacro instance, shared, and the context, not shared, is improperly orchestrated for concurrency. The webmacro instance is local to WMServlet. You might consider synchronizing access to this variable as a trial fix in your local calls below. This will introduce some serialization of web processing but it might help to verify the solution.<br> <br> </font></blockquote> <br> I will try this tip<br> <br> <blockquote cite=3D"mid...@dm..." type=3D"cite"><font style=3D"font-family: arial,helvetica,sans-serif;" size=3D"2">One thing you might consider trying is to use a different cache handler. A few years ago, I wrote one and it is a part of the distro. I have never had a problem using this cache handler.<br> <br> Lane<br> </font></blockquote> <br> <br> Thank you !<br> <blockquote cite=3D"mid...@dm..." type=3D"cite"><font style=3D"font-family: arial,helvetica,sans-serif;" size=3D"2"><br> --- Nikolaos Papadakis <a class=3D"moz-txt-link-rfc2396E" href=3D"mailto:= nk...@cc..."><nk...@cc...></a> wrote:<br> <br> From: Nikolaos Papadakis <a class=3D"moz-txt-link-rfc2396E" href=3D"mailt= o:nk...@cc..."><nk...@cc...></a><br> Date: Sat, 18 Feb 2006 10:50:33 +0200<br> To: <a class=3D"moz-txt-link-abbreviated" href=3D"mailto:webmacro-user@li= sts.sourceforge.net">web...@li...</a><br> Subject: [WebMacro-user] [WebMacro-user<br> <br> Hi all, <br> <br> I am using webmacro to develop a web-based application. <br> Things seemed to work fine till yesterday. A customer complained that he <br> managed to =E2=80=9Csee=E2=80=9D the account of another customer when he = logged-in using <br> his own credentials (he send me a screen shot of the =E2=80=9Cview=E2=80=9D= ). <br> By inspecting the log file I found that both users have been logged-in <b= r> at the same time (the second customer logged in 3 sec after the first <br= > customer). <br> The system has more than 1000 customers but this inconvenience happened <br> for the first time yesterday (?). <br> It seems that different sessions have been mixed up. <br> The servlet includes a handle method and uses several other methods <br> (methods that write and read from a database). <br> All methods are defined in the same class (that extends WMServlet). <br> As far as I know servlets must not have instance variables (only local <b= r> variables -inside methods- to prevent data corruption and <br> inconsistencies). However in my servlet I use only one non local <br> variable (a Logger) and I don't think this is the cause of the problem. <br> As an attached file I send you a fragment of the before mentioned servlet. <br> Can instance variables be defined in the case of a servlet that extends <br> WMServlet? (I mean is it safe? or its not a good practice without <br> keeping precautions?) <br> Do you have any idea what can have caused this inconvenience ? <br> Is the use of methods inside the servlet thread-safe ? <br> <br> Please help!!! <br> Nikolaos Papadakis <br> </font>------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! <a class=3D"moz-txt-link-freetext" href=3D"http://sel.as-us.falkag.net/se= l?cmd=3Dk&kid=103432&bid#0486&dat=121642">http://sel.as-us.falkag.net/sel= ?cmd=3Dk&kid=103432&bid#0486&dat=121642</a> _______________________________________________ Webmacro-user mailing list <a class=3D"moz-txt-link-abbreviated" href=3D"mailto:Webmacro-user@lists.= sourceforge.net">Web...@li...</a> <a class=3D"moz-txt-link-freetext" href=3D"https://lists.sourceforge.net/= lists/listinfo/webmacro-user">https://lists.sourceforge.net/lists/listinf= o/webmacro-user</a> </blockquote> </body> </html> |