David Sean Taylor wrote: >>>I suspect that the proxies are causing double the objects to >>> >>> >>be created >> >> >>>per request. Im still not convinced that this is a good >>> >>> >>solution to the >> >> >>>security issue. (please convince me :) >>> >>> >>> >>Things should be first correct, and then fast. >> >>- With this approach it is easier to get it correct. >>- Modern VMs should not have problem with frequent creation of >>lightweight objects >>- From my inspection of logs, it looks like turbine ACL are >>not cached. >> >> > >I am in the middle of removing the Turbine ACL checks in the security_14 >branch and replacing them with Jetspeed permission checks. >Hope to have something working in a few days. > > I will take a look to the branch differences ASAP (after catching email and looking at changes with current CVS).
> > >>I think this is the real performance bottleneck, as we must go to the >>database for each request. I don't know (yet) whose fault it is. >>- As a "reasoning by example", tomcat uses a HTTPRequest object >>internally (with setters and getters), and a final HTTPFacade >>class that >>encapsulates the "true" one, and which is the only one that >>the servlet >>sees. This is a typical security strategy. They don't use the Proxy >>class. I'm not sure whether they pool these objects, but we >>could when >>the implementation is functional (not right now). This is where I got >>"the way" :-) >> >> > >I agree that correctness is the first priority. >That's why I am proposing the use of Java proxies, so that we can >correctly proxy the objects. > > As I said before, we are correctly proxying the objects, except that some code assumes more than it should about the received objects. This is a typical problem with bean-like environments like velocity, which are not strongly typed. The fact that we are seeing these problems now is not a fault of the proxy, but rather of previous code, which used undocumented (un-contracted) methods becauses they needed them, instead of promoting them to the contract (the Portlet API) or looking for a different way to get the functionality. If we use a "lazy" approach WRT types we are hiding further the problem, which is that the current contracts are not good, or have missing pieces. Having a restrictive implementation of proxies is, IMO, good, as it enforces the API and points to its failures. >The time for performance analysis will come next. Its long overdue. > > > >>>If we must use proxies, they should be Java proxies. >>>Of course this requires 1.3. Anyone have problems with that? >>> >>> >>> >>> >>I'm not so sure. We rather should clean our interfaces, so that the >>security implementation is not a mess. Having three orthogonal >>interfases that a Portlet can implement does not look right. Making >>"final" the classes will enable Hotspot inlining of method calls in a >>security conscious way. >> >> > >Sounds like you are -1 on using the standard Java proxy class then. > > I have mixed feelings on this one. I think there are two separate issues here: - having portlets implement three orthogonal interfaces (Portlet, Cacheable, PortletState) looks bad. Cacheable was deprecated by Raphael a long time ago, but it is still there. PortletState looks like it should be mostly part of the Container contracts (the minimized, maximized, etc. part is largely independent of the portlet itselt), or go straight into the Portlet API, so that all Portlets *must* implement it. This is the real reason that makes a jdk1.2 implementation of proxies bad. This, and the fact that we are relying on calls that are not really part of the Portlet API, or casting Portlet to AbstractPortlet, both are bad programming practices. This implementation can not and should not be blamed for this. On the proxy class: - I feel that dropping jdk1.2 compliance now is not good. A lot of people can be running Jetspeed on jdk1.2 yet, due to administrative requirements. I'm not even sure that my customers can migrate to jdk1.3 right now. It would be great, though. - I am not sure about how efficient this implementation is. Using reflection used to be much slower than the other approach. - As I said, the polimorphic interfaces are not a good feature, and it would be better to solve the separate issue first. So, I would be more confortable with a cleaner set of interfaces and a jdk1.2 implementation. But I don't really disregard a cleaner implementation of proxies using the jdk1.3 feature. > > >>>>The only bad part would be that process for non-logged in >>>>users would be >>>>(I hope slightly) slower. But a portal needs security, and >>>> >>>> >>I think we >> >> >>>>could have handled this by optimising the security path once the >>>>implementation was right. >>>> >>>> >>>> >>>> >>You don't comment on this one, but it again makes the >>security code much >>cleaner and enables the administration of anonymous permits >>much in the >>same way as other users (user "guest", role "guest", for instance). >> >> > >Sorry, Im +1 on having an anonymous user in the db. > > OK. This change, I think, never went into the CVS. I will review it in the light of the code changes. This will make security code much cleaner (no isLoggedIn() branches, no special cases, better code in general). Regards, Santiago >David > > -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
