Singleton的问题!!help!!

07-12-08 scorptio
最近在做一些代码安全性检查,使用了Fortify。发现了一些曾经引以为豪的代码既然会出现高危的错误。希望各位达人给予帮助。

设计初衷:共享WebApplicationContext,通过getBean(String beanName)取得对应的bean处理相应的业务逻辑。 public class BaseAction extends Action {

private WebApplicationContext wac;

protected ServletContext context;

public void setServlet(ActionServlet actionServlet) {

super.setServlet(actionServlet);

context = actionServlet.getServletContext();

wac = WebApplicationContextUtils.getRequiredWebApplicationContext(context);

}

protected Object getBean(String beanName) {

return wac.getBean(beanName);

}

} 高危错误代码: context = actionServlet.getServletContext();

wac = WebApplicationContextUtils.getRequiredWebApplicationContext(context);

错误原因分析:BaseAction继承自Action,单例实现。该类含有属性:wac与context,在多线程环境下不能保证这两个属性的合理逻辑。于是掉入了一个单例模式的陷阱。(请各位达人完善)

解决方法:等待中...(请各位达人帮助)

power1128
2007-12-08 15:52
如果是Struts1的话,它的Action的确是单态模式,也就是对n多request(请求的Action相同)只会用一个Action对象来响应,所以Action里不应该定义实例变量.

Struts2(或者说是Webwork)就不是单态模式,你那样做就没有问题

你这个工具挺有意思,很好,很强大.改天下来看看

[该贴被power1128于2007-12-08 15:54修改过]

scorptio
2007-12-08 17:40
请大家多多帮助!

由于检测出高危错误,以至项目无法提交客户.

希望各位给出最佳的解决方案.目前的解决方法是将wac与context设置为static变量,虽然不会再有高危的错误报告,可是总是感觉不大对劲..

1.系统中各模块的Action都继承于BaseAction.setServlet方法会在每次加载各模块Action子类的时候调用,定义成static会不会有什么问题??

2.单例中不应该出现有状态的局部变量\有状态的全局变量\无状态的局部变量.而只能出现无状态的全局变量,这句话是否正确?

banq
2007-12-10 10:08
写的方法是高危,但是看你的方法内容,则没有关系。

你的方法内容是获得一个Web项目的一个单例,实际就是SPring容器,而且大部分是读,而不是读写并加。

单例陷阱我已经在以前帖子中说了,只有在单例+写+同步等操作下才可能死锁,因为代码不可控性,今天你注意没这么做,不代表其他无知的人不这么做,所以,这种singlton会被严谨的工具认为是高危,这是正确的。

scorptio
2007-12-10 21:15
谢谢 banq!

如果需要消除这段高危代码,需要如何重构.

因为只是存在读操作,我是否可以把wac与context定义为static(这样工具不会报错)

请问这样改会不会有问题?

banq
2007-12-11 10:51
不能用static,这样Spring容器中所有资源都不能随着你的WEB项目消失而消失。

主要问题在于你的setServlet(ActionServlet actionServlet)这个方法,这其实是一个初始化工作,试验在BaseAction的构造方法中首先实现,这样表示有前有后,可能不会报高危错误。

实在不行,做一个懒加载的方法,在这个方法中,检查一下wac是否为空,如果为,就进行初始化,不为空,就直接返回wac值。

scorptio
2007-12-12 18:56
1.主要问题在于你的setServlet(ActionServlet actionServlet)这个方法,这其实是一个初始化工作,试验在BaseAction的构造方法中首先实现,这样表示有前有后,可能不会报高危错误。

在构造函数中无法实现.因为wac的初始化需要用到actionServlet参数.在构造函数中应该无法取到ActionServlet吧?

2.实在不行,做一个懒加载的方法,在这个方法中,检查一下wac是否为空,如果为,就进行初始化,不为空,就直接返回wac值。

按照您的意思我修改代码如下:可是仍然有高危错误.

public class BaseAction extends Action {

private WebApplicationContext wac;

protected ServletContext context;

public void setServlet(ActionServlet actionServlet){

super.setServlet(actionServlet);

if(wac==null){

context = actionServlet.getServletContext();

wac = WebApplicationContextUtils.getRequiredWebApplicationContext(context);

}

}

protected Object getBean(String beanName) {

return wac.getBean(beanName);

}

}

错误报告如下:

ABSTRACT

Servlet member fields may allow one user to see another user's data.

EXPLANATION

Many Servlet developers do not understand that, unless a Servlet implements the SingleThreadModel interface, the Servlet is a singleton; there is only one instance of the Servlet, and that single instance is used and re-used to handle multiple requests that are processed simultaneously by different threads.

A common result of this misunderstanding is that developers use Servlet member fields in such a way that one user may inadvertently see another user's data. In other words, storing user data in Servlet member fields introduces a data access race condition.

Example 1: The following Servlet stores the value of a request parameter in a member field and then later echoes the parameter value to the response output stream.

public class GuestBook extends HttpServlet {

String name;

protected void doPost (HttpServletRequest req,

HttpServletResponse res) {

name = req.getParameter("name");

...

out.println(name + ", thanks for visiting!");

}

}

While this code will work perfectly in a single-user environment, if two users access the Servlet at approximately the same time, it is possible for the two request handler threads to interleave in the following way:

Thread 1: assign "Dick" to name

Thread 2: assign "Jane" to name

Thread 1: print "Jane, thanks for visiting!"

Thread 2: print "Jane, thanks for visiting!"

Thereby showing the first user the second user's name.

RECOMMENDATIONS

Do not use Servlet member fields for anything but constants. (i.e. make all member fields static final).

Developers are often tempted to use Servlet member fields for user data when they need to transport data from one region of code to another. If this is your aim, consider declaring a separate class and using the Servlet only to "wrap" this new class.

Example 2: The bug in the example above can be corrected in the following way:

public class GuestBook extends HttpServlet {

protected void doPost (HttpServletRequest req,

HttpServletResponse res) {

GBRequestHandler handler = new GBRequestHandler();

handler.handle(req, res);

}

}

public class GBRequestHandler {

String name;

public void handle(HttpServletRequest req,

HttpServletResponse res) {

name = req.getParameter("name");

...

out.println(name + ", thanks for visiting!");

}

}

Alternatively, a Servlet can implement the SingleThreadModel interface, in which case the Servlet container will maintain a pool of Servlet objects and dispatch a different object to process each request. Depending on the container implementation and the needs of the application, using the SingleThreadModel interface may cause significant performance problems.

REFERENCES

[1] The Java Servlet Specification, Sun Microsystems, http://java.sun.com/products/servlet/download.html

INSTANCE ID: 3D9FCE421944A87E40229CF99BAA0E1C

RULE ID: 9818E2BB-8E28-4CBE-88CD-DE8DF5EFF040

SCA CONFIDENCE: 5.0

banq
2007-12-13 14:00
晕,去除setServlet方法,在getBean加入,如下,如果还不行,就每次老老实实的用WebApplicationContextUtils获得Spring实例:

protected Object getBean(String beanName) {

if(wac==null){

context = actionServlet.getServletContext();

wac = WebApplicationContextUtils.getRequiredWebApplicationContext(context);

}

return wac.getBean(beanName);

}

猜你喜欢