Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader WebappClassLoader.java

2005-09-28 Thread Jan Luehe
Remy,

Remy Maucherat wrote On 09/28/05 03:12,:
 [EMAIL PROTECTED] wrote:
 
  +/*
  + * Clear the IntrospectionUtils cache.
  + *
  + * Implementation note:
  + * Any reference to IntrospectionUtils which may cause the static
  + * initalizer of that class to be invoked must occur prior to 
 setting
  + * the started flag to FALSE, because the static initializer of
  + * IntrospectionUtils makes a call to
  + * org.apache.commons.logging.LogFactory.getLog(), which ultimately
  + * calls the loadClass() method of the thread context classloader,
  + * which is the same as this classloader, whose impl throws a
  + * ThreadDeath if the started flag has been set to FALSE.
  + */
  +IntrospectionUtils.clear();
  +
 
 
 This commit does not make sense to me. The static initializer is called 
 when loading the class, and obviously the webapp CL is not going to load 
 IntrospectionUtils.

This code in IntrospectionUtils.java:

private static org.apache.commons.logging.Log log=
org.apache.commons.logging.LogFactory.getLog(
IntrospectionUtils.class );

will cause LogFactoryImpl.getLogConstructor() to be called, which
in turn will invoke LogFactoryImpl.loadClass(String name), which is
implemented as follows:

private static Class loadClass( final String name )
throws ClassNotFoundException
{
Object result = AccessController.doPrivileged(
new PrivilegedAction() {
public Object run() {
ClassLoader threadCL = getContextClassLoader();
if (threadCL != null) {
try {
return threadCL.loadClass(name);
} catch( ClassNotFoundException ex ) {
// ignore
}
}
try {
return Class.forName( name );
} catch (ClassNotFoundException e) {
return e;
}
}
});

if (result instanceof Class)
return (Class)result;

throw (ClassNotFoundException)result;
}

Notice the use of the thread context classloader (to load the class
with the given name), which is the same as the WebappClassLoader.

WebappClassLoader.loadClass() has this:

// Don't load classes if class loader is stopped
if (!started) {
log.info(sm.getString(webappClassLoader.stopped));
throw new ThreadDeath();
}

We have seen the ThreadDeath in our callstacks, hence this fix.

Jan


 You should forget that the CVS exists, as we're in the middle of 
 migrating to SVN (of course, losing that commit will not be a problem).
 
 Rémy
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader WebappClassLoader.java

2005-09-28 Thread Jan Luehe
Remy,

Remy Maucherat wrote On 09/28/05 10:18,:
 Jan Luehe wrote:
 
We have seen the ThreadDeath in our callstacks, hence this fix.
 
 
 Nobody is reading what I am writing anymore ...

No, I did.

 I wrote:
 The static initializer is called when loading the class, and obviously 
 the webapp CL is not going to load IntrospectionUtils.
 
 IntrospectionUtils will be loaded once, and its static initializer run 
 once.

Yes, but with lazy resolution, it will be loaded when the
IntrospectionUtils symbol is first encountered, which may
be inside WebappClassLoader.stop().

IntrospectionUtils' static initializer will cause an invocation of
loadClass() on the thread's context classloader, which corresponds to
the WebappClassLoader, whose loadClass() throws ThreadDeath when
its started flag has been set to FALSE.

Therefore, we must avoid referencing IntrospectionUtils in
WebappClassLoader.stop() after the started flag has been
set to FALSE.

 So I am ok with your fix, but I don't understand how it can occur 
 in regular Tomcat.

It's probably not occurring in standalone Tomcat, but only
in embedded Tomcat. In standalone Tomcat, IntrospectionUtils is
probably getting resolved (and its static initializer invoked)
prior to calling WebappClassLoader.stop(),
whereas in embedded mode, IntrospectionUtils is first referenced
and loaded by WebappClassLoader.stop().

 The big comment block is quite pointless, as it tries to be 
 informational, but doesn't correspond to reality (I am personally 
 against this kind of commit message duplication comment).

Sure, I just thought this line might be an easy candidate for
being moved around if there was no comment.

 As a reminder, CVS shound't be used anymore.

Yes.


Jan

 
 Rémy
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: DO NOT REPLY [Bug 36534] - Context relative URLs returned by ServletContext.getResource() for the same path are not equal

2005-09-07 Thread Jan Luehe


Remy Maucherat wrote:
 [EMAIL PROTECTED] wrote:
 
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=36534.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=36534


[EMAIL PROTECTED] changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|WONTFIX |




--- Additional Comments From [EMAIL PROTECTED]  2005-09-07 19:47 ---
Good point about toString(). I have a solution for that as well. Just override
org.apache.naming.resources.DirContextURLStreamHandler.toExternalForm() and 
have
it ignore the authority part of the URL, as follows (this is copied from
java.net.URLStreamHandler.toExternalForm(), with authority part ignored):

/**
 * Converts a codeURL/code of a specific protocol to a
 * codeString/code.
 *
 * @param   u   the URL.
 * @return  a string representation of the codeURL/code argument.
 */
protected String toExternalForm(URL u) {

  // pre-compute length of StringBuffer
  int len = u.getProtocol().length() + 1;
  if (u.getPath() != null) {
  len += u.getPath().length();
  }
  if (u.getQuery() != null) {
  len += 1 + u.getQuery().length();
  }
  if (u.getRef() != null) 
  len += 1 + u.getRef().length();

  StringBuffer result = new StringBuffer(len);
  result.append(u.getProtocol());
result.append(:);
if (u.getPath() != null) {
result.append(u.getPath());
}
if (u.getQuery() != null) {
result.append('?');
result.append(u.getQuery());
}
  if (u.getRef() != null) {
  result.append(#);
result.append(u.getRef());
  }
  return result.toString();
}

It is important that URLs returned by ServletContext.getResource() that are
equal have equals() return TRUE. This works for all other kinds of URLs.
 
 
 I hadn't noticed you were the one who filed the bug. Besides skipping 
 the result.append(:); 

Not sure I understand: the : following the protocol is necessary,
so that the printable representation of the context generated URLs
starts with jndi:.

 you should simply apply the fix, it's a very 
 good solution.

I'm still having problems with CVS, which are preventing me from
committing my fix.

If you can commit this on my behalf, I'd appreciate it.
The complete fix consists of the above diff and the following patch
in org.apache.catalina.core.ApplicationContext:

 try {
 resources.lookup(path);
 return new URL
-(jndi, null, 0, getJNDIUri(hostName, fullPath),
- new DirContextURLStreamHandler(resources));
+(jndi, , 0, getJNDIUri(hostName, fullPath),
+ new DirContextURLStreamHandler(resources));
 } catch (Exception e) {
 // Ignore
 }

Thanks,

Jan



 Rémy
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler Generator.java

2005-07-21 Thread Jan Luehe
Bill,

[EMAIL PROTECTED] wrote:
 billbarker2005/07/20 20:59:10
 
   Modified:jasper2/src/share/org/apache/jasper/compiler Generator.java
   Log:
   Make certain that release is called for custom tags when tag-pooling is 
 disabled.
   
   Fix for Bug #35696
   
   Revision  ChangesPath
   1.241 +9 -2  
 jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/Generator.java
   
   Index: Generator.java
   ===
   RCS file: 
 /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/Generator.java,v
   retrieving revision 1.240
   retrieving revision 1.241
   diff -u -r1.240 -r1.241
   --- Generator.java  5 Apr 2005 23:14:43 -   1.240
   +++ Generator.java  21 Jul 2005 03:59:10 -  1.241
   @@ -2278,15 +2278,19 @@
out.printin(if ();
out.print(tagHandlerVar);
out.println(
   -.doEndTag() == javax.servlet.jsp.tagext.Tag.SKIP_PAGE));
   +.doEndTag() == javax.servlet.jsp.tagext.Tag.SKIP_PAGE) 
 {);
out.pushIndent();
   +if(!n.implementsTryCatchFinally()) {
   +out.printin(tagHandlerVar);
   +out.println(.release(););
   +}

I believe the above 4 added lines need to be replaced with this:

+if (!n.implementsTryCatchFinally()) {
+
+if (isPoolingEnabled) {
+out.printin(n.getTagHandlerPoolName());
+out.print(.reuse();
+out.print(tagHandlerVar);
+out.println(););
+} else {
+out.printin(tagHandlerVar);
+out.println(.release(););
+}
+}


Jan


if (isTagFile || isFragment) {
out.printil(throw new SkipPageException(););
} else {
out.printil((methodNesting  0) ? return true; : 
 return;);
}
out.popIndent();
   -
   +out.printil(});
// Synchronize AT_BEGIN scripting variables
syncScriptingVars(n, VariableInfo.AT_BEGIN);

   @@ -2317,6 +2321,9 @@
out.print(.reuse();
out.print(tagHandlerVar);
out.println(););
   +} else {
   +out.printin(tagHandlerVar);
   +out.println(.release(););
}

if (n.implementsTryCatchFinally()) {
   
   
   
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



StandardContext and serialization

2005-06-18 Thread Jan Luehe
Has anybody ever tried serializing a StandardContext?

StandardContext implements java.io.Serializable, but many
of its fields are not. I assume StandardContext has to
be serializable so it can be transmitted (e.g., via RMI)
to any remote javax.management.NotificationListener
(e.g., MapperListener) that gets notified of the context's JMX
registration.

I've started marking some of the non-serializable fields
as transient and reinitialize them in readObject(), but there
are so many direct and indirect references to non-serializable classes
that I'm afraid I'm opening a can of worms.


Jan


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



StandardContext and serialization

2005-06-18 Thread Jan Luehe
[Resending as first attempt seemed to have failed]

Has anybody ever tried serializing a StandardContext?

StandardContext implements java.io.Serializable, but many
of its fields are not. I assume StandardContext has to
be serializable so it can be transmitted (e.g., via RMI)
to any remote javax.management.NotificationListener
(e.g., MapperListener) that gets notified of the context's JMX
registration.

I've started marking some of the non-serializable fields
as transient and reinitialize them in readObject(), but there
are so many direct and indirect references to non-serializable classes
that I'm afraid I'm opening a can of worms.


Jan


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/webapps/docs changelog.xml

2005-05-12 Thread Jan Luehe


[EMAIL PROTECTED] wrote:
 remm2005/05/12 06:01:05
 
   Modified:jasper2/src/share/org/apache/jasper/servlet
 JspCServletContext.java
webapps/docs changelog.xml
   Log:
   - 34465: jspc without web.xml.
   - Submitted by Yoichi Hirose.
   
   Revision  ChangesPath
   1.4   +7 -1  
 jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/servlet/JspCServletContext.java
   
   Index: JspCServletContext.java
   ===
   RCS file: 
 /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/servlet/JspCServletContext.java,v
   retrieving revision 1.3
   retrieving revision 1.4
   diff -u -r1.3 -r1.4
   --- JspCServletContext.java 17 Mar 2004 19:23:05 -  1.3
   +++ JspCServletContext.java 12 May 2005 13:01:04 -  1.4
   @@ -235,7 +235,13 @@
if (!path.startsWith(/))
throw new MalformedURLException(Path ' + path +
' does not start with '/');
   -return (new URL(myResourceBaseURL, path.substring(1)));
   +URL url = new URL(myResourceBaseURL, path.substring(1));
   +if (file.equals(url.getProtocol())) {
   +if (!(new File(url.getFile())).exists()) {
   +return null;
   +}
   +}
   +return url;

}

I don't think this is very efficient. Normally, the resource
with the given path will exist. It is just in the case
of web.xml that it may not exist.

Why not check specifically for existence of web.xml, as follows:


Index: JspConfig.java
===
RCS file:
/home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/JspConfig.java,v
retrieving revision 1.18
diff -u -r1.18 JspConfig.java
--- JspConfig.java  24 Mar 2005 04:08:01 -  1.18
+++ JspConfig.java  13 May 2005 00:09:22 -
@@ -16,6 +16,7 @@

 package org.apache.jasper.compiler;

+import java.io.File;
 import java.io.InputStream;
 import java.util.Iterator;
 import java.util.Vector;
@@ -63,10 +64,12 @@

 try {
 URL uri = ctxt.getResource(WEB_XML);
-if (uri == null) {
+if (uri == null
+|| (file.equals(uri.getProtocol())
+ !(new File(uri.getFile())).exists())) {
// no web.xml
 return;
-   }
+}

 is = uri.openStream();
 InputSource ip = new InputSource(is);


Jan




Jan


   
   
   
   1.308 +7 -0  jakarta-tomcat-catalina/webapps/docs/changelog.xml
   
   Index: changelog.xml
   ===
   RCS file: /home/cvs/jakarta-tomcat-catalina/webapps/docs/changelog.xml,v
   retrieving revision 1.307
   retrieving revision 1.308
   diff -u -r1.307 -r1.308
   --- changelog.xml   11 May 2005 21:39:41 -  1.307
   +++ changelog.xml   12 May 2005 13:01:04 -  1.308
   @@ -153,6 +153,10 @@
default encoding. A side effect of this fix is that the bodies of 
 POST requests that
require FORM authentication are now buffered and made available 
 after a sucessful login. (markt)
  /fix
   +  fix
   +bug34840/bug: Better handling of external WARs redeployment, 
 and ignore docBase specified
   +in context file if within the Host appBase (remm)
   +  /fix
/changelog
  /subsection
  
   @@ -199,6 +203,9 @@
bug34652/bug: Add the ability to get SMAPs when precompiling, 
 submitted by
Daryl Robbins (remm)
  /update
   +  fix
   +bug34465/bug: Jspc failure if there is no web.xml, submitted 
 by Yoichi Hirose (remm)
   +  /fix
/changelog
  /subsection
  
   
   
   
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: Reminder: 5.5.9 tomorrow

2005-03-25 Thread Jan Luehe


Yoav Shapira wrote:
 Hi,
 
 This is just a reminder that I plan to cut and tag Tomcat version 5.5.9
 tomorrow, Saturday March 26th, at 1400h my time, which is 1900h UTC/GMT.
 
  
 
 Jan, as Remy and you discussed yesterday on the mailing list, please revert
 your commit that throws the IllegalStateException in HttpSession#getId.

Done! Will revisit for 5.5.10.

Thanks,

Jan


  
 
 Thanks, and have a good weekend everyone,
 
  
 
 Yoav Shapira
 
 System Design and Management Fellow
 
 MIT Sloan School of Management / School of Engineering
 
 Cambridge, MA USA
 
  mailto:[EMAIL PROTECTED] [EMAIL PROTECTED] /
 mailto:[EMAIL PROTECTED] [EMAIL PROTECTED]
 
  
 
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session StandardSession.java

2005-03-23 Thread Jan Luehe
Yoav,

Yoav Shapira wrote:
 Hola,
 
 
My believe is that the above errata will be reflected in the next
(maintenance) release of the servlet spec. I will remind the servlet
spec lead that this needs to happen.

Jan
 
 
 What's the ETA on this maintenance spec release?

work is supposed to start sometime this summer,
and will be completed in time for J2EE 5.0 FCS.

Jan


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session StandardSession.java

2005-03-22 Thread Jan Luehe


Remy Maucherat wrote:
 Bill Barker wrote:
 
- Original Message - From: Remy Maucherat [EMAIL PROTECTED]
To: Tomcat Developers List tomcat-dev@jakarta.apache.org
Sent: Tuesday, March 22, 2005 1:57 AM
Subject: Re: cvs commit: 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session 
StandardSession.java



[EMAIL PROTECTED] wrote:


billbarker2005/03/21 19:50:03

  Modified:catalina/src/share/org/apache/catalina/session
StandardSession.java
  Log:
  From the comments for R1.11, it seems that some early version of 
the spec had an exception here.  However, this didn't survive to the 
final spec version, so we can once again allow access to 
getLastAccessedTime from an invalid session.
  Fix for Bug #34107


I didn't know that, as the javadoc actually says:

getLastAccessedTime

public long getLastAccessedTime()

Returns the last time the client sent a request associated with 
this session, as the number of milliseconds since midnight January 1, 
1970 GMT, and marked by the time the container received the request.

Actions that your application takes, such as getting or setting a 
value associated with the session, do not affect the access time.

Returns:
a long representing the last time the client sent a request 
associated with this session, expressed in milliseconds since 1/1/1970 
GMT
Throws:

   java.lang.IllegalStateException - if this method is called

on an invalidated session

Which paragraph in the spec has the conflicting statement ?


It's the java-doc for jakarta-servletapi-5 that is wrong.  The javadocs 
in the servlet spec do not contain a 'throws' clause.  See also: 
http://java.sun.com/j2ee/1.4/docs/api/javax/servlet/http/HttpSession.html#getLastAccessedTime()
 
 
 
 Here: 
 http://jcp.org/aboutJava/communityprocess/maintenance/jsr154/154errata.txt
 
 I now read:
 2. Fix typos in HttpSession.java
 
 Replace:
 
   * @exeption IllegalStateException if this method is called on an
   * invalidated session
 
 in javadoc of getId() and getLastAccessedTime()
 With:
 
   * @exception IllegalStateException if this method is called on an
   *  invalidated session
 
 This fix causes the following addition to HttpSession.getId() and
 HttpSession.getLastAccessedTime() methods. Since these two methods are
 already implemented in all containers to throw this exception and the
 spec clearly intends to do so, this is not changing the spec.
 
 SRV.15.1.7.1 Page 267, 268
 
   Throws:
   IllegalStateException - if this method is called on an invalidated
   session
 
 There's highly conflicting information ...


I think the intention is for both methods to throw an ISE in case
they're being called on an invalidated session, as has been clarified
by the errata.

It seems this has been the intent for both methods from the very
beginning, except that the throws clauses in the javadocs at
  http://java.sun.com/j2ee/1.4/docs/api/javax/servlet/http/HttpSession.html

which are also copied in the servlet spec, were never generated for
these 2 methods because of a stupid typo (@exeption vs @exception) in
the HttpSession.java source file.

The HttpSession.java source in jakarta-servletapi-5 already has the fix.

My believe is that the above errata will be reflected in the next
(maintenance) release of the servlet spec. I will remind the servlet
spec lead that this needs to happen.


Jan






 Rémy
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm RealmBase.java

2005-03-02 Thread Jan Luehe
Bill/Remy,

Bill Barker wrote:
 - Original Message -
 From: Remy Maucherat [EMAIL PROTECTED]
 To: Tomcat Developers List tomcat-dev@jakarta.apache.org
 Sent: Wednesday, March 02, 2005 11:56 AM
 Subject: Re: cvs commit:
 jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm
 RealmBase.java
 
 
 
[EMAIL PROTECTED] wrote:

luehe   2005/03/02 11:27:11

  Modified:catalina/src/share/org/apache/catalina/realm
 
 RealmBase.java
 
  Log:
  Consider the case where original request was mapped to welcome page.
  In this case, the mapped welcome page (and not the original request
  URI!) needs to be the target of hasResourcePermission().

  This is consistent with the change that had been made in
 
 findSecurityConstraints().
 
  BTW, shouldn't request.getDecodedRequestURI() return the mapped
  welcome page (instead of the original URI) in this case?
  In other words, shouldn't the path passed to
mappingData.requestPath.setString(pathStr)
  in Mapper.java be propagated to the request object associatd with the
  mappingData?

I consider welcome files to be internal forwards (since it is allowed to
handle them this way). As a result, they shouldn't be matched by
secrurity constraints. Only the original request path should be the used
(so here it's getDecodedRequestURI - as sent by the client).

 
 
 I agree with Remy.  It's an internal Tomcat implementation detail that
 welcome-files aren't handled via DefaultServlet doing:
   RequestDispatcher rd = request.getRequestDispatcher(welcome[i]);
   rd.forward(request, response);
 Since this is explicitly allowed by the spec, nobody can expect that a
 security-constraint mapped only to the welcome-file will be applied.
 However, this is probably another thing that should be better specified in
 the 2.5 spec.


But SRV.9.10 (Welcome Files) already has this:

  The container may send the request to the welcome resource with
  a forward, a redirect, or a container specific mechanism
  **that is indistinguishable from a direct request**.

The latter to me implies that any sec constraints must be applied
to the mapped welcome page (if any).

Also, see the attached diffs, in particular:

-String uri = request.getDecodedRequestURI();
-String contextPath = hreq.getContextPath();
-if (contextPath.length()  0)
-uri = uri.substring(contextPath.length());
+String uri = request.getRequestPathMB().toString();

in findSecurityConstraints().

When accessing host:port:/somecontext/,
which has welcome page /somecontext/index.jsp,

request.getDecodedRequestURI() returns /somecontext/,
whereas request.getRequestPathMB().toString() returns
/index.jsp (as set by the mapper), so there already is a precedent
in findSecurityConstraints() to match sec constraints against
welcome page, which I think makes sense.

Otherwise, the following sec constraint:

  security-constraint
web-resource-collection
  web-resource-nameProtected Area/web-resource-name
  url-pattern*.jsp/url-pattern
  http-methodPUT/http-method
  http-methodDELETE/http-method
  http-methodGET/http-method
  http-methodPOST/http-method
/web-resource-collection
auth-constraint
  role-nametomcat/role-name
/auth-constraint
  /security-constraint

which is supposed to protect all JSP pages, would be bypassed if a
request was mapped to index.jsp welcome page.


Jan



 
Rémy

 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 
 
 
 
 
 This message is intended only for the use of the person(s) listed above as 
 the intended recipient(s), and may contain information that is PRIVILEGED and 
 CONFIDENTIAL.  If you are not an intended recipient, you may not read, copy, 
 or distribute this message or any attachment. If you received this 
 communication in error, please notify us immediately by e-mail and then 
 delete all copies of this message and any attachments.
 
 In addition you should be aware that ordinary (unencrypted) e-mail sent 
 through the Internet is not secure. Do not send confidential or sensitive 
 information, such as social security numbers, account numbers, personal 
 identification numbers and passwords, to us via ordinary (unencrypted) e-mail.
 
 
 
 
 
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
Index: RealmBase.java
===
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm/RealmBase.java,v
retrieving revision 1.23
retrieving revision 1.24
diff -u -r1.23 -r1.24
--- RealmBase.java  26 Dec 2003 17:33:44 -  1.23
+++ RealmBase.java  10 Jan 2004 17:23:39 -  1.24
@@ -1,7 +1,7 @@
 

Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm RealmBase.java

2005-03-02 Thread Jan Luehe
Remy,

Remy Maucherat wrote:
 Jan Luehe wrote:
 
Bill/Remy,

But SRV.9.10 (Welcome Files) already has this:

  The container may send the request to the welcome resource with
  a forward, a redirect, or a container specific mechanism
  **that is indistinguishable from a direct request**.

The latter to me implies that any sec constraints must be applied
to the mapped welcome page (if any).
 
 
 The plot thickens.


What do you mean by that? ;-)
Do you agree the spec is pretty clear about the fact that
any sec constraints must be applied to welcome page?


Also, see the attached diffs, in particular:

-String uri = request.getDecodedRequestURI();
-String contextPath = hreq.getContextPath();
-if (contextPath.length()  0)
-uri = uri.substring(contextPath.length());
+String uri = request.getRequestPathMB().toString();

in findSecurityConstraints().

When accessing host:port:/somecontext/,
which has welcome page /somecontext/index.jsp,

request.getDecodedRequestURI() returns /somecontext/,
whereas request.getRequestPathMB().toString() returns
/index.jsp (as set by the mapper), so there already is a precedent
in findSecurityConstraints() to match sec constraints against
welcome page, which I think makes sense.
 
 
 Right. However, when I made that commit, the current mapper behavior may 
 not have been in place already, or maybe it's simply that I thought the 
 two would be equivalent (I was busy optimizing at the time). I don't 
 quite remember ;)


I think you did the right thing without realizing it. :)
The change I committed earlier today is just consistent with
what you had done.

I'm still nervous about request.getDecodedRequestURI() returning
the original URI even after the request has been mapped to a welcome
page. This violates spec requirement that any container specific
mechanism for mapping request to welcome page must be
indistinguishable from a direct request.


Jan



 Rémy
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm RealmBase.java

2005-03-02 Thread Jan Luehe
Bill,

Bill Barker wrote:
 - Original Message -
 From: Jan Luehe [EMAIL PROTECTED]
 To: Tomcat Developers List tomcat-dev@jakarta.apache.org
 Sent: Wednesday, March 02, 2005 12:51 PM
 Subject: Re: cvs commit:
 jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm
 RealmBase.java
 
 
 
Bill/Remy,

Bill Barker wrote:

- Original Message -
From: Remy Maucherat [EMAIL PROTECTED]
To: Tomcat Developers List tomcat-dev@jakarta.apache.org
Sent: Wednesday, March 02, 2005 11:56 AM
Subject: Re: cvs commit:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm
RealmBase.java




[EMAIL PROTECTED] wrote:


luehe   2005/03/02 11:27:11

 Modified:catalina/src/share/org/apache/catalina/realm

RealmBase.java


 Log:
 Consider the case where original request was mapped to welcome page.
 In this case, the mapped welcome page (and not the original request
 URI!) needs to be the target of hasResourcePermission().

 This is consistent with the change that had been made in

findSecurityConstraints().


 BTW, shouldn't request.getDecodedRequestURI() return the mapped
 welcome page (instead of the original URI) in this case?
 In other words, shouldn't the path passed to
   mappingData.requestPath.setString(pathStr)
 in Mapper.java be propagated to the request object associatd with the
 mappingData?

I consider welcome files to be internal forwards (since it is allowed to
handle them this way). As a result, they shouldn't be matched by
secrurity constraints. Only the original request path should be the used
(so here it's getDecodedRequestURI - as sent by the client).



I agree with Remy.  It's an internal Tomcat implementation detail that
welcome-files aren't handled via DefaultServlet doing:
  RequestDispatcher rd = request.getRequestDispatcher(welcome[i]);
  rd.forward(request, response);
Since this is explicitly allowed by the spec, nobody can expect that a
security-constraint mapped only to the welcome-file will be applied.
However, this is probably another thing that should be better specified
 
 in
 
the 2.5 spec.


But SRV.9.10 (Welcome Files) already has this:

 The container may send the request to the welcome resource with
 a forward, a redirect, or a container specific mechanism
 **that is indistinguishable from a direct request**.

 
 
 I read the emphasised text as referring to 'container specific mechanism'.

So do I. indistinguishable from a direct request means that any
sec constraints will have to be applied to welcome pages when the
request is sent to the welcome page via container specific
mechanism (as in Tomcat).

 Yes, I agree that the last-minute changes that were made to 9.10 made it a
 total mess, but it still explicitly allows DefaultServlet to do a
 rd.forward.

Yes, in which case the welcome page that is the target of the
rd.forward() will not be subjected to any sec constraints.

So the spec is inconsistent as to whether sec constraints need to
be applied to welcome pages.

This means that web developers should always use a pattern of this
form:

  url-pattern/*/url-pattern

in their DD's security constraints if they want their welcome
pages to be subjected to the specified sec constraints no matter
which container their webapp is deployed on.

If they specify:

  url-pattern*.jsp/url-pattern

their index.jsp welcome page will not be subjected to any sec
constraints in containers that send the request to the welcome page
using rd.forward().

The latter to me implies that any sec constraints must be applied
to the mapped welcome page (if any).

Also, see the attached diffs, in particular:

 
 
 Firstly, I'm strongly -1 on the patch, since removing the 'if(found)return'
 statements causes Tomcat to no longer be spec-complient.  Just because the
 spec is silly doesn't mean that we don't have to implement it.

The patch I attached has been 1 year old.
My main purpose in attaching it was to draw attention to
this change in rev 1.24:
 
 
-String uri = request.getDecodedRequestURI();
-String contextPath = hreq.getContextPath();
-if (contextPath.length()  0)
-uri = uri.substring(contextPath.length());
+String uri = request.getRequestPathMB().toString();

in findSecurityConstraints().

Remy had restored the 'if(found)return' in rev 1.25:

revision 1.25
date: 2004/01/11 09:23:42;  author: remm;  state: Exp;  lines: +11 -11
- Ooops. Put back the if(found) blocks.

revision 1.24
date: 2004/01/10 17:23:39;  author: remm;  state: Exp;  lines: +16 -11
- findMethod wasn't called on the right collection.
- The algorithm ignored extension mapped constraints as long as a widcard
  or exact mapped constraint was found. This doesn't seem right (I did
quickly
  read the relevant portions of the spec).
- Next, I'll try to optimize the algorithm (allocating a collection on
each request
  is not good, we should add a matched contraints array on the request).

When accessing host:port:/somecontext

Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/valves ValveBase.java

2005-03-02 Thread Jan Luehe


Remy Maucherat wrote:
 [EMAIL PROTECTED] wrote:
 
luehe   2005/02/23 11:27:56

  Modified:catalina/src/share/org/apache/catalina/authenticator
FormAuthenticator.java NonLoginAuthenticator.java
SSLAuthenticator.java SingleSignOn.java
   catalina/src/share/org/apache/catalina/realm
DataSourceRealm.java JDBCRealm.java JNDIRealm.java
RealmBase.java UserDatabaseRealm.java
   catalina/src/share/org/apache/catalina/valves ValveBase.java
  Log:
  No change in functionality.
  
  Added new containerLog instance var to RealmBase and ValveBase, which is
  initialized as container.getLogger() inside setContainer().
  
  This will make it easier to do something like
  
containerLog = LogFactory.getLog(container.logName()+.RealmBase);
  
  in the future, as suggested by Bill Barker.
 
 
 Actually, this is probably a bad idea (or at least the implementation is 
 bad): the logger must be retrieved only when the context class loader of 
 the application is set.

This is where I'm not following: ;-)
I don't see any dependency on the context's context class loader when
calling ContainerBase.getLogger(), which is implemented like this:

  logger = LogFactory.getLog(logName());

A context's context class loader is required only for loading webapp
resources.

Jan


 This means no retrieving the logger in 
 ValveBase.setContainer, since this is first called in StandardContext().



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/valves ValveBase.java

2005-03-02 Thread Jan Luehe


Remy Maucherat wrote:
 Jan Luehe wrote:
 
Remy Maucherat wrote:


Actually, this is probably a bad idea (or at least the implementation is 
bad): the logger must be retrieved only when the context class loader of 
the application is set.

This is where I'm not following: ;-)
I don't see any dependency on the context's context class loader when
calling ContainerBase.getLogger(), which is implemented like this:

  logger = LogFactory.getLog(logName());

A context's context class loader is required only for loading webapp
resources.
 
 
 The configuration for that logger should be doable at the webapp level 
 (to summarize, I'd like to be able to have my logger configured using 
 either /WEB-INF/classes/logging.properties or 
 common/classes/logging.properties if the first one does not exist). The 
 logging implementation gets which config it should lookup for the logger 
 based on what the context CL is when calling getLogger. So the context 
 CL needs to be set properly (or getLogger has to be called again).

Got it. Thanks!

Jan




-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm DataSourceRealm.java JAASCallbackHandler.java JAASMemoryLoginModule.java JDBCRealm.java JNDIRealm.java RealmBase.java UserDatabaseRealm.java

2005-02-18 Thread Jan Luehe


Remy Maucherat wrote:
 [EMAIL PROTECTED] wrote:
 
luehe   2005/02/18 11:17:57

  Modified:catalina/src/share/org/apache/catalina/realm
DataSourceRealm.java JAASCallbackHandler.java
JAASMemoryLoginModule.java JDBCRealm.java
JNDIRealm.java RealmBase.java
UserDatabaseRealm.java
  Log:
  More logging cleanup
 
 
 I disagree with these changes: all these logs are application specific, 
 which is why I direct them to the application category.
 
 Can you explain why it is bad ?

I think the boundaries between container and application specific logs
has been pretty blurred, and we've been using the two inconsistently in
the past.

In my interpretation, any log messages issued by ServletContext.log()
should be directed to container.getLogger(), but any container
generated log messages should be directed to LogFactory.getLog().

Where do you see the line? This has been something that has confused
me in the past.


Jan


 Rémy
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm DataSourceRealm.java JAASCallbackHandler.java JAASMemoryLoginModule.java JDBCRealm.java JNDIRealm.java RealmBase.java UserDatabaseRealm.java

2005-02-18 Thread Jan Luehe


Remy Maucherat wrote:
 Jan Luehe wrote:
 
Remy Maucherat wrote:


[EMAIL PROTECTED] wrote:



luehe   2005/02/18 11:17:57

Modified:catalina/src/share/org/apache/catalina/realm
  DataSourceRealm.java JAASCallbackHandler.java
  JAASMemoryLoginModule.java JDBCRealm.java
  JNDIRealm.java RealmBase.java
  UserDatabaseRealm.java
Log:
More logging cleanup


I disagree with these changes: all these logs are application specific, 
which is why I direct them to the application category.

Can you explain why it is bad ?


I think the boundaries between container and application specific logs
has been pretty blurred, and we've been using the two inconsistently in
the past.

In my interpretation, any log messages issued by ServletContext.log()
should be directed to container.getLogger(), but any container
generated log messages should be directed to LogFactory.getLog().

Where do you see the line? This has been something that has confused
me in the past.
 
 
 The thing has been blurred in 5.0. In 4.1, loggers were associated with 
 containers (ex: a context). All logging for the container (including all 
 subcomponents such as realm, manager, etc) would go to it. I liked that 
 better.
 
 I reintroduced that in 5.5 by adding a per container category (with 
 nesting).

I'm still confused. ;-)
Which log messages are supposed to go to LogFactory.getLog(), and which
ones to Container.getLogger()?
For example, in StandardContext.java, we're using LogFactory.getLog()
exclusively. Shouldn't most of them also be considered app specific
and therefore be directed to Container.getLogger()?

I understand the difference between the 2 Log types is that one
carries the app name and the container nesting levels in its name,
whereas the other carries the fully qualified class name of the
container class that issued the log message.

Therefore, while the former allows log messages to be differentiated
by app name in the server log, the latter allows to pinpoint the
container component that printed the log message. I think a combination
of the two would be most useful.

I also think it would make sense to be able to distinguish the log
messages issued by ServletContext.log() from those log messages
that originate from the container.

I'll undo my 2 previous commits until I have a better understanding
of the issue.

Thanks,


Jan


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/realm DataSourceRealm.java JAASCallbackHandler.java JAASMemoryLoginModule.java JDBCRealm.java JNDIRealm.java RealmBase.java UserDatabaseRealm.java

2005-02-18 Thread Jan Luehe
Remy,

Remy Maucherat wrote:
 Jan Luehe wrote:
 
Remy Maucherat wrote:

I'm still confused. ;-)
Which log messages are supposed to go to LogFactory.getLog(), and which
ones to Container.getLogger()?
For example, in StandardContext.java, we're using LogFactory.getLog()
exclusively. Shouldn't most of them also be considered app specific
and therefore be directed to Container.getLogger()?
 
 
 It's a little risky. We should use the container logger only while the 
 classloader is initialized.
 
 It's probably far from perfect ;)

Then how about RealmBase.authenticate()?

RealmBase.authenticate(String username, String credentials)
uses Container.getLogger(), whereas the other RealmBase.authenticate()
methods use LogFactory.getLog(). Shouldn't this be consistent?
This is where my confusion has stemmed from.

If you agree, I can help make the inconsistent cases consistent.

 In parallel, I want to start a commons component about a j.u.logging 
 implementation keyed per classloader (as seen in bug 33143; it may be 
 expanded a little later to allow per classloader configuration).
 
 I'm looking for committers for the proposal :)

Sounds interesting!

Jan


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core StandardContext.java

2005-02-17 Thread Jan Luehe


Remy Maucherat wrote:
 [EMAIL PROTECTED] wrote:
 

  This patch should fix the issue.
  
  Let me know if you see any problems.
 
 
 There's the same code (and more) in WebappClassLoader.stop():
 
  // Clear the classloader reference in common-logging
  org.apache.commons.logging.LogFactory.release(this);
  // Clear the classloader reference in the VM's bean introspector
  java.beans.Introspector.flushCaches();
 
 IMO, it's the right place to do this kind of cleaning up in the 
 classloader, since it's CL related.

You're absolutely right. You had fixed this in TC long time ago
(revision 1.20). This commit did slip through the cracks, so our
internal version was still suffering from this leak.

Thanks!

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/startup ContextConfig.java LocalStrings.properties

2005-02-14 Thread Jan Luehe
Remy,

Remy Maucherat wrote:
 [EMAIL PROTECTED] wrote:

luehe   2005/02/14 11:27:41

  Modified:catalina/src/share/org/apache/catalina/startup
ContextConfig.java LocalStrings.properties
  Log:
  Use configuration from alt-dd if specified.
  (Setter for alt-dd had been added to StandardContext, but this info
was never used.)


 What's the use of that ? I see it could be useful to deviate from the
 standard, but that's it ...

Yes, this is useful for when your web module is deployed as part of a
J2EE application inside a J2EE container. According to J2EE.8.3.1
(Assembling a J2EE Application), step 3:

  The deployment descriptors for the J2EE modules must be edited to link
  internally satisfied dependencies and eliminate any redundant security
  role names. An optional element alt-dd [...] may be used when it is
  desirable to preserve the original deployment descriptor. The element
  alt-dd specifies an alternate deployment descriptor to use at
  deployment time.

 I suppose the field was lying around from TC 4.0, and not actually used,
 right ?

According to cvs, this StandardContext property was added in
TOMCAT_5_0_1.


Jan




-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties

2004-11-01 Thread Jan Luehe
Remy Maucherat wrote:
 [EMAIL PROTECTED] wrote:
 
 
luehe   2004/11/01 14:38:44

 Modified:catalina/src/share/org/apache/catalina/connector
   RequestFacade.java LocalStrings.properties
 Log:
 Throw more meaningful exception (instead of NPE) if underlying request has been
 recycled and attempt is made to access it via its facade

 
 I think I always consistently refused this change (no use: if people who 
 hack can't be bothered to look that up in the code, then I don't think 
 they'll understand what your exception really means either), but I'll 
 give up on that one.

In this case, it's useful because rather than instinctively filing a bug
against Tomcat when seeing a NPE, people will be reminded to check their
code first, because they're obviously using Tomcat in a way it was not
intended to be used.

We just ran into this internally.


Jan


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties

2004-11-01 Thread Jan Luehe
Bill Barker wrote:
 - Original Message -
 From: Jan Luehe [EMAIL PROTECTED]
 To: Tomcat Developers List [EMAIL PROTECTED]
 Sent: Monday, November 01, 2004 3:41 PM
 Subject: Re: cvs commit:
 jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
 RequestFacade.java LocalStrings.properties
 
 
 
Remy Maucherat wrote:

[EMAIL PROTECTED] wrote:



luehe   2004/11/01 14:38:44

Modified:catalina/src/share/org/apache/catalina/connector
  RequestFacade.java LocalStrings.properties
Log:
Throw more meaningful exception (instead of NPE) if underlying request
 
 has been
 
recycled and attempt is made to access it via its facade


I think I always consistently refused this change (no use: if people who
hack can't be bothered to look that up in the code, then I don't think
they'll understand what your exception really means either), but I'll
give up on that one.

In this case, it's useful because rather than instinctively filing a bug
against Tomcat when seeing a NPE, people will be reminded to check their
code first, because they're obviously using Tomcat in a way it was not
intended to be used.

 
 
 I agree with Remy:  It's totally unnecessary, and gives somebody reading the
 code that the request can be null.  The javadocs should probably be updated
 with something like:
   * @exception IllegalStateException If you are a total moron without a clue
 ;-)

In the case I was referring to, some project was storing a
servlet request (facade) in a ThreadLocal and, due to a bug in their
code, was hanging on to it beyond the request's lifetime. This was
happening only under rare circumstances.

So in this case, the Request behind the facade was indeed null.


Jan





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector CoyoteAdapter.java

2004-10-28 Thread Jan Luehe
Bill,

Bill Barker wrote:
 [EMAIL PROTECTED] wrote in message 
 news:[EMAIL PROTECTED]
 
luehe   2004/10/27 15:58:17

 +
 +private Method[] getAllDeclaredMethods(Class c) {
 +
 +if (c.equals(javax.servlet.http.HttpServlet.class)) {
 +return null;
 +}
 +
 +Method[] parentMethods = 
getAllDeclaredMethods(c.getSuperclass());
 
 
 If the servlet isn't a HttpServlet (e.g. it's a JSP page) then this will 
 recurse down to j.l.Object, when c.getSuperClass will return 'null', and you 
 will get an NPE from the 'c.equals' line.

Actually, in the case of a JSP, we're dealing w/ JspServlet, which is an
instance of HttpServlet.

I've changed the code to return a constant set of methods if the servlet
is not an instance of HttpServlet, avoiding the NPE. :)

Thanks,

Jan


 IMHO, this patch is an overly complex way to try and determine something 
 that isn't determinable under the servlet spec (again, think JSP page :). 
 You might as well just set the Allow header to any old constant set of 
 methods.
 
 
 
 
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector CoyoteAdapter.java

2004-10-28 Thread Jan Luehe

Actually, in the case of a JSP, we're dealing w/ JspServlet, which is an
instance of HttpServlet.

I've changed the code to return a constant set of methods if the servlet
is not an instance of HttpServlet, avoiding the NPE. :)

 
 
 My bad.  I should pay more attention to Jasper :).
 
 However, a JSP page still will return 'Allow: OPTIONS' after all of this
 work :).

Yes, but you'll get a more useful reply for any custom HttpServlets. ;-)
Notice that what the patch does is return the same methods that OPTIONS
would have returned in the Allow response header, minus the disabled
TRACE.

Since the HTTP spec requires that the Allow header should reflect
the capabilities of the requested resource, I think it is a good idea
to follow it as much as we can. It's not like we're adding tons of
code in order to achieve this. :)


Jan


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/security SecurityUtil.java

2004-10-25 Thread Jan Luehe
Bill Barker wrote:
 - Original Message -
 From: [EMAIL PROTECTED]
 To: [EMAIL PROTECTED]
 Sent: Monday, October 25, 2004 2:18 PM
 Subject: cvs commit:
 jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/security
 SecurityUtil.java
 
 
 
  @@ -251,18 +251,17 @@
   if (session != null){
   subject =

 
 (Subject)session.getAttribute(Globals.SUBJECT_ATTR);
 
  -}

  -if (subject == null){
  -subject = new Subject();
  +if (subject == null){
  +subject = new Subject();

  -if (principal != null){
  -subject.getPrincipals().add(principal);
  +if (principal != null){
  +subject.getPrincipals().add(principal);
  +}
  +
  +session.setAttribute(Globals.SUBJECT_ATTR,
 
 subject);
 
   }
   }
  -
  -if (session != null)
  -session.setAttribute(Globals.SUBJECT_ATTR,
 
 subject);
 
   }

   Subject.doAsPrivileged(subject, pea, null);
 
 
 With this patch, If there is no session defined, then 'subject' will be null
 when I get to the doAsPrivieged.

Good catch! Fixed so that Subject is created regardless of whether
session exists, but it is added to the session only if the session
did not already contain any.


Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core ApplicationHttpResponse.java

2004-10-15 Thread Jan Luehe
Remy/Bill,

Remy Maucherat wrote:
 [EMAIL PROTECTED] wrote:
 
 
luehe   2004/10/14 17:00:35

 Modified:catalina/src/share/org/apache/catalina/core
   ApplicationHttpResponse.java
 Log:
 Expose any errors on an included resource.
 
 For example, a JSP with this include action:
   jsp:include page=nonexistent
 or
   jsp:include page=nonexistent.jsp
 where nonexistent[.jsp] does not exist, currently returns silently, hiding the
 fact that the resource to be included does not exist.
 
 This patch returns a 404 with the name of the nonexistent resource.
 
 Yes, SRV.8.3 (The Include Method) mentions that
 
   it [the target servlet] cannot set headers or call any method that
   affects the headers of the response. Any attempt to do so must be
   ignored.
 
 but i don't think it is referring to the error case.
 
 Let me know if you see any problems.  

 
 Same as Bill. It would be logical that it refers to the whole HTTP 
 header(all of its elements already are), since the idea is that it might 
 already have been sent.

I agree the above section needs to be clarified.

 It's true that we have no good way of reporting problems when including. 
 This has always been a problem from a user perspective.

OK, do you think it would be a better idea to return a null
RequestDispatcher in this case?

In ApplicationContext.getRequestDispatcher(path), we could check if the
wrapper that the given path is being mapped to corresponds to the
JspServlet or DefaultServlet, and only in that case, call getResource()
to make sure the requested resource actually exists. If it doesn't,
return null.

I really don't like the idea of not reporting anything to the user.

Jan


 Rémy
 
 
 -
 To unsubscribe, e-mail: [EMAIL PROTECTED]
 For additional commands, e-mail: [EMAIL PROTECTED]
 



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: TCKs for 5.5.3 and 5.0.29

2004-10-06 Thread Jan Luehe
Hi Amy,
Amy Roh wrote:
I ran the Servlet/JSP TCKs.  They both passed on the 5.0.29 release.
The Servlet TCK passed on the 5.5.3 release but the following 2 JSP TCK 
tests failed out of 615 tests.
Test case throws exception: [BaseUrlClient] null failed! Check output 
for cause of failure.
 a.. 
com/sun/ts/tests/jsp/spec/core_syntax/implicitobjects/URLClient.java#checkConfigTest 
: URLClient_checkConfigTest
 b.. 
com/sun/ts/tests/jsp/spec/tagfiles/implicitobjects/URLClient.java#checkConfigTest 
: URLClient_checkConfigTest
I committed a fix for these 2 failures yesterday (in
StandardWrapper.java):
  revision 1.49
  date: 2004/10/06 00:54:46;  author: luehe;  state: Exp;  lines: +1 -1
  Undid previous change, as in the case where a servlet has a jsp-file
  and also declares some init params, as in:
servlet
  servlet-namexxx/servlet-name
  jsp-file/xxx.jsp/jsp-file
  init-param
param-namename1/param-name
param-valuevalue1/param-value
  /init-param
/servlet
  it needs its *own* JspServlet instance that it can initialize with its
  own params. Sharing of JspServlet instance is not possible in this
  case.
  Will have to come up with a better solution against loss of monitoring
  info (the JspServlet that handles the above jsp-file currently is not
  registered with JMX).
I think we need to retag this file with 5.5.3.
Jan

Thanks,
Amy
- Original Message - From: Shapira, Yoav [EMAIL PROTECTED]
To: Tomcat Developers List [EMAIL PROTECTED]
Sent: Wednesday, October 06, 2004 6:28 AM
Subject: TCKs for 5.5.3 and 5.0.29

Hi,
Sun folks, or anyone with access to the Servlet and JSP TCKs: can you
please run them against the 5.5.3 and 5.0.29 releases when you get a
chance, and post your results here?  Thanks in advance ;)
Both 5.0.29 and 5.5.3 pass all our internal tests without exception.
Yoav Shapira
Millennium Research Informatics


This e-mail, including any attachments, is a confidential business 
communication, and may contain information that is confidential, 
proprietary and/or privileged.  This e-mail is intended only for the 
individual(s) to whom it is addressed, and may not be saved, copied, 
printed, disclosed or used by anyone else.  If you are not the(an) 
intended recipient, please immediately delete this e-mail from your 
computer system and notify the sender.  Thank you.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/webapps/docs changelog.xml jasper-howto.xml

2004-10-04 Thread Jan Luehe
Hi Remy,
[EMAIL PROTECTED] wrote:
remm2004/10/04 10:39:46
  Modified:jasper2/src/share/org/apache/jasper/resources
LocalStrings.properties
   jasper2/src/share/org/apache/jasper
EmbeddedServletOptions.java Options.java JspC.java
   jasper2/src/share/org/apache/jasper/compiler Compiler.java
   webapps/docs changelog.xml jasper-howto.xml
  Log:
  - Allow configuring the interval following a compilation during which a JSP will not 
be checked for modifications.
how is this different from the 'checkInterval' option?
Jan

  Revision  ChangesPath
  1.2   +2 -1  jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/resources/LocalStrings.properties
  
  Index: LocalStrings.properties
  ===
  RCS file: /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/resources/LocalStrings.properties,v
  retrieving revision 1.1
  retrieving revision 1.2
  diff -u -r1.1 -r1.2
  --- LocalStrings.properties	1 Sep 2004 10:08:48 -	1.1
  +++ LocalStrings.properties	4 Oct 2004 17:39:45 -	1.2
  @@ -143,6 +143,7 @@
   jsp.warning.sendErrToClient=Warning: Invalid value for the initParam sendErrToClient. Will use the default value of \false\
   jsp.warning.classDebugInfo=Warning: Invalid value for the initParam classdebuginfo. Will use the default value of \false\
   jsp.warning.checkInterval=Warning: Invalid value for the initParam checkInterval. Will use the default value of \300\ seconds
  +jsp.warning.modificationTestInterval=Warning: Invalid value for the initParam modificationTestInterval. Will use the default value of \4000\ milliseconds
   jsp.warning.development=Warning: Invalid value for the initParam development. Will use the default value of \true\
   jsp.warning.fork=Warning: Invalid value for the initParam fork. Will use the default value of \true\
   jsp.warning.reloading=Warning: Invalid value for the initParam reloading. Will use the default value of \true\
  
  
  
  1.14  +24 -4 jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/EmbeddedServletOptions.java
  
  Index: EmbeddedServletOptions.java
  ===
  RCS file: /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/EmbeddedServletOptions.java,v
  retrieving revision 1.13
  retrieving revision 1.14
  diff -u -r1.13 -r1.14
  --- EmbeddedServletOptions.java	2 Sep 2004 16:05:06 -	1.13
  +++ EmbeddedServletOptions.java	4 Oct 2004 17:39:45 -	1.14
  @@ -164,7 +164,12 @@
*/
   private String javaEncoding = UTF8;
   
  -/*
  +/**
  + * Modification test interval.
  + */
  +public int modificationTestInterval = 4000;
  +
  +/**
* Is generation of X-Powered-By response header enabled/disabled?
*/
   private boolean xpoweredBy;
  @@ -226,6 +231,13 @@
   }
   
   /**
  + * Modification test interval.
  + */
  +public int getModificationTestInterval() {
  +return modificationTestInterval;
  +}
  +
  +/**
* Is Jasper being used in development mode?
*/
   public boolean getDevelopment() {
  @@ -450,6 +462,17 @@
   }
   }
   
  +String modificationTestInterval = config.getInitParameter(modificationTestInterval);
  +if (modificationTestInterval != null) {
  +try {
  +this.modificationTestInterval = Integer.parseInt(modificationTestInterval);
  +} catch(NumberFormatException ex) {
  +if (log.isWarnEnabled()) {
  +log.warn(Localizer.getMessage(jsp.warning.modificationTestInterval));
  +}
  +}
  +}
  +
   String development = config.getInitParameter(development);
   if (development != null) {
   if (development.equalsIgnoreCase(true)) {
  @@ -589,9 +612,6 @@
   	}
   }
   
  -/*
  - * X-Powered-By
  - */
   String xpoweredBy = config.getInitParameter(xpoweredBy); 
   if (xpoweredBy != null) {
   if (xpoweredBy.equalsIgnoreCase(true)) {
  
  
  
  1.25  +6 -0  jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/Options.java
  
  Index: Options.java
  ===
  RCS file: /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/Options.java,v
  retrieving revision 1.24
  retrieving revision 1.25
  diff -u -r1.24 -r1.25
  --- Options.java	2 Sep 2004 16:05:06 -	1.24
  +++ Options.java	4 Oct 2004 17:39:45 -	1.25
  @@ -164,4 +164,10 @@
* Are Text strings to be generated as char arrays?
*/
   public boolean genStringAsCharArray();
  +
  +/**
  + * Modification test interval.
  + */
  +public int 

Re: cvs commit: jakarta-tomcat-catalina/webapps/docs changelog.xml jasper-howto.xml

2004-10-04 Thread Jan Luehe
Remy Maucherat wrote:
Jan Luehe wrote:
Hi Remy,
[EMAIL PROTECTED] wrote:
remm2004/10/04 10:39:46
  Modified:jasper2/src/share/org/apache/jasper/resources
LocalStrings.properties
   jasper2/src/share/org/apache/jasper
EmbeddedServletOptions.java Options.java 
JspC.java
   jasper2/src/share/org/apache/jasper/compiler 
Compiler.java
   webapps/docs changelog.xml jasper-howto.xml
  Log:
  - Allow configuring the interval following a compilation during 
which a JSP will not be checked for modifications.

how is this different from the 'checkInterval' option?

The check interval is in seconds, and is for the background reloading 
thread. This means at most one check every X ms.

I did consider reusing it, but since the unit was different, I chose not 
to.
I think this new option is going to confuse users.
If we wanted to reduce the number of last-modified checks in
development mode, we should at least try to leverage the existing
option and use the same unit. The default for the new option
(4000ms) seems to imply that seconds may be a reasonable unit
for it as well.
Also, I think it is more intuitive if we check for last modification
date on each access in dev mode. I don't think perf improvements are
important in dev mode.
  +listrongmodificationTestInterval/strong - If development has 
to be set to
  +codetrue/code for any reason (such as dynamic generation of 
JSPs), setting
  +this to a high value will improve performance a lot./li

Why won't dynamic generation of JSPs work in nondev mode? Notice that
in JspServleWrapper, we compile if
  options.getDevelopment() || firstTime
Jan

Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/webapps/docs changelog.xml jasper-howto.xml

2004-10-04 Thread Jan Luehe

I feel strongly about this. Dev mode is the default, and is needed for 
some special purpose stuff (where JSPs are regenerated on the fly), so 
it need to perform relatively well.

IMO, the background reloading thread is way too complex and buggy. It 
should go in favor of a much simpler mechanism (if you really want only 
one thing; personally, I would keep both, as it's more flexible).

  +listrongmodificationTestInterval/strong - If development has 
to be set to
  +codetrue/code for any reason (such as dynamic generation of 
JSPs), setting
  +this to a high value will improve performance a lot./li

Why won't dynamic generation of JSPs work in nondev mode? Notice that
in JspServleWrapper, we compile if
  options.getDevelopment() || firstTime

It works for the first compilation, of course. But most people who use 
that obvious refresh their JSPs.

Overall, I think that having two attributes is appropriate.
OK, I think I've bought your point.
Can we specify the interval in seconds then, or what was the reason
to specify it in ms? Do you think a JSP could change that frequently?
When you specify a value 1000ms, you might as well specify -1.
Also, we should add this option to the documentation of the
JspServlet init params in the default web.xml.
Jan
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/xmlparser XMLEncodingDetector.java

2004-10-01 Thread Jan Luehe
Remy Maucherat wrote:
Remy Maucherat wrote:
Is it a standalone class or something like this ? If it is - copy 
paste into the Jasper code and remove the funky loading logic.

(I thought JDK 1.5 was supposed to include the same Xerces 2 we were 
using - what happened ?) 

Follow up: the Xerces packages got renamed to 
com.sun.org.apache.xerces.internal.*.

So we have two options, one which I like and one which I don't like:
a) Write a XercesJava5EncodingDetector class with the right classnames
b) As the 5 classes used by the XercesEncodingDetector are self 
contained, we could copy them to the o.a.jasper.xmlparser package, so 
that Jasper can be spec compliant even without Xerces installed, merge 
the content of XercesEncodingDetector into XMLEncodingDetector, and 
remove the dynamic loading of Xerces

Since I think it's not acceptable to require Xerces to be compliant, I 
would choose b).
Agreed. Will make these changes.
Jan
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/xmlparser XMLEncodingDetector.java

2004-09-30 Thread Jan Luehe
Remy Maucherat wrote:
[EMAIL PROTECTED] wrote:
luehe   2004/09/30 14:57:09
 Modified:jasper2/src/share/org/apache/jasper/xmlparser
   XMLEncodingDetector.java
 Log:
 Log warning when XML prolog cannot be parsed.
 
 The 2 JSP TCK failures reported by Jeanfrancois are due to this, with
 this root cause:
 
   java.lang.ClassNotFoundException: org.apache.xerces.util.SymbolTable
 

We don't bundle Xerces anymore with the binaries, as Java 5 is supposed 
to be used. With JRE 1.4, the compat package must be added (it adds JMX 
and Xerces as endorsed).
I think this would explain the problem.
Yes, it does. I yet have to find XML prolog parsing and encoding
autodetection support in JDK 1.5 (the equivalent of what Xerces used to
provide).
Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/authenti cator SingleSignOn.java

2004-09-20 Thread Jan Luehe
Brian,
Brian Stansberry wrote:
Jan,
At 06:18 PM 9/16/2004 +, you wrote:
luehe   2004/09/16 11:18:41
 Modified:catalina/src/share/org/apache/catalina/authenticator
   SingleSignOn.java
 Log:
 - Removed deregister(String ssoid), because it is no longer needed
   (used to be called when session was logged out, which is no longer
   supported)
I'm not sure what you meant here by no longer supported.  Do you mean 
the cross-webapp signout feature that deregister(String ssoid) provided, 
or has there been some more fundamental change in TC's handling of 
HttpSession.invalidate()?
I was referring to the removal of javax.servlet.http.HttpSession.logout(),
which had been added temporarily to Servlet 2.4 and was later removed
before the spec went final. See this log entry in the history of
javax.servlet.http.HttpSession:
  revision 1.3
  date: 2003/04/07 21:27:36;  author: jfarcand;  state: Exp;  lines: +0
  -15
  As required by the upcoming Servlet spec 2.4 PFD 3, remove the
  logout() method.
This method was the only method that generated a SessionEvent of
type SESSION_DESTROYED_EVENT with event data equal to logout, which
used to invalidate all remaining sessions (if any) associated with
the SingleSignOn entry.
In either case, the SingleSignOn docs at 
http://jakarta.apache.org/tomcat/tomcat-5.5-doc/config/host.html#Single%20Sign%20On 
need to be updated, as they state:

As soon as the user logs out of one web application (for example, by 
invalidating or timing out the corresponding session if form based login 
is used), the user's sessions in all web applications will be 
invalidated. Any subsequent attempt to access a protected resource in 
any application will require the user to authenticate himself or herself 
again.

(Actually, that paragraph was incorrect even before this patch, since 
the time out of a session would not cause other sessions to be 
invalidated.)

Agreed. I'm going to remove this paragraph.
Jan

Best,
Brian Stansberry
 - Replaced call to removeSession(String, Session) with
   deregister(String, Session), which is identical, and removed
   removeSession(String, Session) because it is no longer needed
 Revision  ChangesPath
 1.18  +3 -92 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/authenticator/SingleSignOn.java 

 Index: SingleSignOn.java
 ===
 RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/authenticator/SingleSignOn.java,v 

 retrieving revision 1.17
 retrieving revision 1.18
 diff -u -r1.17 -r1.18
 --- SingleSignOn.java 29 Aug 2004 16:46:09 -  1.17
 +++ SingleSignOn.java 16 Sep 2004 18:18:41 -  1.18
 @@ -287,24 +287,10 @@
  synchronized (reverse) {
  ssoId = (String) reverse.get(session);
  }
 -if (ssoId == null)
 +if (ssoId == null) {
  return;
 -
 -// Was the session destroyed as the result of a timeout?
 -// If so, we'll just remove the expired session from the
 -// SSO.  If the session was logged out, we'll log out
 -// of all session associated with the SSO.
 -if ((session.getMaxInactiveInterval()  0)
 - (System.currentTimeMillis() - 
session.getLastAccessedTime() =
 -session.getMaxInactiveInterval() * 1000)) {
 -removeSession(ssoId, session);
 -} else {
 -// The session was logged out.
 -// Deregister this single session id, invalidating
 -// associated sessions
 -deregister(ssoId);
  }
 -
 +deregister(ssoId, session);
  }

 @@ -468,46 +454,6 @@
  /**
 - * Deregister the specified single sign on identifier, and 
invalidate
 - * any associated sessions.
 - *
 - * @param ssoId Single sign on identifier to deregister
 - */
 -protected void deregister(String ssoId) {
 -
 -if (container.getLogger().isDebugEnabled())
 -container.getLogger().debug(Deregistering sso id ' + 
ssoId + ');
 -
 -// Look up and remove the corresponding SingleSignOnEntry
 -SingleSignOnEntry sso = null;
 -synchronized (cache) {
 -sso = (SingleSignOnEntry) cache.remove(ssoId);
 -}
 -
 -if (sso == null)
 -return;
 -
 -// Expire any associated sessions
 -Session sessions[] = sso.findSessions();
 -for (int i = 0; i  sessions.length; i++) {
 -if (container.getLogger().isTraceEnabled())
 -container.getLogger().trace( Invalidating session  
+ sessions[i]);
 -// Remove from reverse cache first to avoid recursion
 -synchronized (reverse) {
 -reverse.remove(sessions[i]);
 -}
 -// Invalidate this session
 -sessions[i].expire();
 -}
 -
 -// NOTE:  Clients may still possess the old single 

Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/authenti cator SingleSignOn.java

2004-09-20 Thread Jan Luehe
Brian Stansberry wrote:
Hi Jan,

At 11:02 PM 9/20/2004 +, you wrote:
luehe   2004/09/16 11:18:41
 Modified:catalina/src/share/org/apache/catalina/authenticator
   SingleSignOn.java
 Log:
 - Removed deregister(String ssoid), because it is no longer needed
   (used to be called when session was logged out, which is no longer
   supported)
I'm not sure what you meant here by no longer supported.  Do you 
mean the cross-webapp signout feature that deregister(String ssoid) 
provided, or has there been some more fundamental change in TC's 
handling of HttpSession.invalidate()?

I was referring to the removal of 
javax.servlet.http.HttpSession.logout(),
which had been added temporarily to Servlet 2.4 and was later removed
before the spec went final. See this log entry in the history of
javax.servlet.http.HttpSession:

  revision 1.3
  date: 2003/04/07 21:27:36;  author: jfarcand;  state: Exp;  lines: +0
  -15
  As required by the upcoming Servlet spec 2.4 PFD 3, remove the
  logout() method.
This method was the only method that generated a SessionEvent of
type SESSION_DESTROYED_EVENT with event data equal to logout, which
used to invalidate all remaining sessions (if any) associated with
the SingleSignOn entry.

The code in sessionEvent() that checked the session's last accessed time 
was intended as a workaround to try to discriminate timeouts from 
intentional logouts after the logout() method was removed from the 
spec.  It was applied as a fix to bug 9077, which complained about the 
SSO valve not invalidating related sessions.  The CVS logs for 
SingleSignOn.java revs 1.7 and 1.11 touch on this and there was also 
some discussion on the dev list last Nov 24.  I'm curious as to why this 
is no longer supported.
OK, my bad. I've restored the previous version. Maybe 
SessionEvent.getData() could have differentiated between session
expiration and session invalidation, so we would not have to determine
the cause of the SESSION_DESTROYED_EVENT by comparing
getLastAccessedTime() and getMaxInactiveInterval()?

In any case, as a result of this, I am going to restore this bullet in 
the Single Sign-On documentation, after removing the part marked by
[REMOVE ...]:

  As soon as the user logs out of one web application (for example, by
  invalidating [REMOVE: or timing out] the corresponding session if form
  based login is used), the user's sessions in all web applications will
  be invalidated. Any subsequent attempt to access a protected resource
  in any application will require the user to authenticate himself or
  herself again.
Thanks!
Jan


Could a config parameter be added to the valve 
to allow this behavior?

_
STOP MORE SPAM with the new MSN 8 and get 2 months FREE* 
http://join.msn.com/?page=features/junkmail

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: JspServlet.serviceJspFile() getResourceAsStream vs getResource

2004-09-08 Thread Jan Luehe
Hi Tim,
Tim Funk wrote:
This is just a nit, but in JspServlet.serviceJspFile() there is ...
synchronized(this) {
wrapper = (JspServletWrapper) rctxt.getWrapper(jspUri);
if (wrapper == null) {
// Check if the requested JSP page exists, to avoid
// creating unnecessary directories and files.
InputStream resourceStream =
context.getResourceAsStream(jspUri);
if (resourceStream == null) {

response.sendError(HttpServletResponse.SC_NOT_FOUND,
   jspUri);
return;
} else {
try {
resourceStream.close();
} catch(IOException e) { /* ignore */ }
}
...
}
}

Is there any reason we are getting the stream instead of checking for a 
null URL, for example:

synchronized(this) {
wrapper = (JspServletWrapper) rctxt.getWrapper(jspUri);
if (wrapper == null) {
// Check if the requested JSP page exists, to avoid
// creating unnecessary directories and files.
if (null==context.getResource(jspUri)) {

response.sendError(HttpServletResponse.SC_NOT_FOUND,
   jspUri);
return;
}
...
}
}

This way the existence of the file is checked instead of opening it.
Good point! There must have been a reason for using getResourceAsStream()
in the past, but I agree it is no longer necessary and just
committed your proposed patch.
Jan


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] [5.5] Release plan votes

2004-08-27 Thread Jan Luehe
Remy Maucherat wrote:
ballot
I approve the release plan:
[X] Yes
[ ] No
/ballot
ballot
Tomcat 5.5 should use the following API set for the coding:
[ ] J2SE 1.3
[X] J2SE 1.4
[ ] J2SE 5.0
/ballot
J2SE 5.0 is in beta right now, and there won't be widespread support
for it for a while, so it's probably safer to stick to J2SE 1.4 for
Tomcat 5.5.
As for Tomcat 6, there might be a possibility that the specs
will require J2SE 5.0, as Jeanfrancois mentioned.
ballot
Yoav Shapira will act as the release manager for this branch:
[X] Yes
[ ] No
/ballot
Jan
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-servletapi-5/jsr154/src/share/dtd web-app_2_4.xsd

2004-08-24 Thread Jan Luehe
Jeanfrancois Arcand wrote:

Petr Jiricka wrote:
Hi,
I remember there was an issue that the default DD in conf/web.xml was 
not valid by the schema - is this change related to that issue?

Yes, this should fix the issue.
Also, with the updated schema, it is now possible to upgrade the
default web.xml to Servlet 2.4, which I just did.
Will commit the same change to TOMCAT_5.
Jan
-- Jeanfrancois
Thanks
Petr
[EMAIL PROTECTED] wrote:
jfarcand2004/08/24 10:01:34
 Modified:jsr154/src/share/dtd web-app_2_4.xsd
 Log:
 Update the web-app_2_4.xsd according
 to the JSR154 maintenance release.
 
   o The restriction facet of mime-typeType should be updated
 from:-
 xsd:pattern value=[\p{L}\-\p{Nd}]+/[\p{L}\-\p{Nd}\.]+/
 to:-
 xsd:pattern value=[^\p{Cc}^\s]+/[^\p{Cc}^\s]+
 
 submitted by: Yutaka Yoshida at sun dot com
 
 Revision  ChangesPath
 1.14  +2 -2  
jakarta-servletapi-5/jsr154/src/share/dtd/web-app_2_4.xsd
 
 Index: web-app_2_4.xsd
 ===
 RCS file: 
/home/cvs/jakarta-servletapi-5/jsr154/src/share/dtd/web-app_2_4.xsd,v
 retrieving revision 1.13
 retrieving revision 1.14
 diff -u -r1.13 -r1.14
 --- web-app_2_4.xsd18 Mar 2004 16:40:34 -1.13
 +++ web-app_2_4.xsd24 Aug 2004 17:01:34 -1.14
 @@ -30,7 +30,7 @@
xsd:annotation
  xsd:documentation
   -  Copyright 2002 Sun Microsystems, Inc., 901 San Antonio
 +  Copyright 2004 Sun Microsystems, Inc., 901 San Antonio
Road, Palo Alto, California 94303, U.S.A. All rights
reserved.
   @@ -804,7 +804,7 @@
xsd:simpleContent
xsd:restriction base=j2ee:string
 -xsd:pattern value=[\p{L}\-\p{Nd}]+/[\p{L}\-\p{Nd}\.]+/
 +  xsd:pattern value=[^\p{Cc}^\s]+/[^\p{Cc}^\s]+/
/xsd:restriction
  /xsd:simpleContent
/xsd:complexType
 
 
 

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
 


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: 5.0.28 next week?

2004-08-24 Thread Jan Luehe
Shapira, Yoav wrote:
Hola,
I'm ready to cut 5.0.28 once the JSP folks are done with their work.  I
think you guys are actually all done and waiting for me, right?  Shall
we say this weekend with Friday as the deadline for committing any code
on the TOMCAT_5_0 branch?
+1.
All Jasper critical bugs, as well upgrading default web.xml to
Servlet 2.4, have been ported.
Thanks!
Jan
Yoav Shapira
Millennium Research Informatics


This e-mail, including any attachments, is a confidential business communication, and 
may contain information that is confidential, proprietary and/or privileged.  This 
e-mail is intended only for the individual(s) to whom it is addressed, and may not be 
saved, copied, printed, disclosed or used by anyone else.  If you are not the(an) 
intended recipient, please immediately delete this e-mail from your computer system 
and notify the sender.  Thank you.
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector Request.java

2004-08-09 Thread Jan Luehe
Remy Maucherat wrote:
Bill Barker wrote:
- Original Message - From: [EMAIL PROTECTED]
To: [EMAIL PROTECTED]
Sent: Thursday, August 05, 2004 6:27 PM
Subject: cvs commit:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
Request.java
 

luehe   2004/08/05 18:27:50
 Modified:catalina/src/share/org/apache/catalina/connector
   Request.java
 Log:
 Avoid allocating SimpleDateFormat[] for each request. Instead, declare
  
SimpleDateFormat[] as static and use static initializer to initialize it.
 

-1.  SimpleDateFormat isn't thread-safe, so you can't have multiple 
threads
accessing them.  That's why they were instance variables, so that one one
thread would ever be accessing them.
OK.
 This is consistent with SimpleDateFormat[] in
  
org.apache.tomcat.util.http.FastHttpDateFormat.
Which is well known to be broken ;-).
 

There's a misunderstanding about the design (I don't think 
FastHttpDateFormat is that broken anyway ;) ). The formats passed in 
argument has to be thread local, and FHDF won't sync.

So -1 too.

actually, FHDF.parseDate() does sync around SimpleDateFormat.parse()
if the threadLocalformats passed to it is null, in which case it
falls back to its own static formats (notice that in this case,
FHDF.internalParseDate() is within the synchronized block):
public static final long parseDate(String value,
   DateFormat[] threadLocalformats) {

if (threadLocalformats != null) {
date = internalParseDate(value, threadLocalformats);
synchronized (parseCache) {
updateCache(parseCache, value, date);
}
} else {
synchronized (parseCache) {
date = internalParseDate(value, formats);
updateCache(parseCache, value, date);
}
}
}
The DateFormat[] passed in from Request.parseDate() is *identical* to
the static formats in FHDF, so why are we not just passing null
from Request.parseDate() and leverage FHDF.formats (instead of having
each Request object provide its own SimpleDateFormat[] instance var)?
Is it to avoid that the potentially expensive date parsing is
performed inside the synchronized block?
Thanks,
Jan
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector Request.java

2004-08-09 Thread Jan Luehe
[EMAIL PROTECTED]
The DateFormat[] passed in from Request.parseDate() is *identical* to
the static formats in FHDF, so why are we not just passing null
from Request.parseDate() and leverage FHDF.formats (instead of having
each Request object provide its own SimpleDateFormat[] instance var)?
Is it to avoid that the potentially expensive date parsing is
performed inside the synchronized block?

Yes, it's sort of obvious ;)
How much memory does a formatter uses ? I'd like to point out that 
pasing of dates will happen relatively often (last modified headers in 
requests, etc).

To summarize:
- passing null is correct, but syncs (otherwise it wouldn't work)
- passing a thread local formatter is correct
- passing a shared formatter isn't correct (as there would be thread 
safety issues)
Agreed.
I stumbled across mem usage as I was running a stress test
under OptimizeIt (and eventually getting OutOfMemory errors from
Servlets), and noticed a large number of char[] instances had been
allocated due to each Request creating its own SimpleDateFormat[].
Jan
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector Request.java

2004-08-09 Thread Jan Luehe
Remy Maucherat wrote:
Jan Luehe wrote:
Agreed.
I stumbled across mem usage as I was running a stress test
under OptimizeIt (and eventually getting OutOfMemory errors from
Servlets), and noticed a large number of char[] instances had been
allocated due to each Request creating its own SimpleDateFormat[].

You get an OOM because of it ? I think not, though.
No, definitely not because of this, but i'm in the process of
eliminating any redundant allocations.
I thought SimpleDateFormat with a constant pattern was a candidate, but
as Bill and you pointed out (and the javadocs now mention), it is not
thread-safe.
You need to speak hard numbers here (because syncing will have its own 
cost, esp on a large server that your company makes so much cash on ;) 
):
Agreed. :)
how many bytes are allocated by each formatter ?
I'll get back to you on this. For the time being, I'll revert my
previous change.
Jan
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: New committer: Peter Rossbach

2004-07-30 Thread Jan Luehe
+1
Jan
Remy Maucherat wrote:
Hi,
I'd like to nominate Peter Rossbach pr _at_ objektpark.de as a 
committer on the Tomcat project. Peter submitted a significant amount of 
useful patches for Tomcat, and wants to contribute more.

He has my +1.
Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-connectors/coyote/src/java/org/apache/coyote Response.java

2004-07-28 Thread Jan Luehe
Bill,

luehe   2004/07/27 17:43:17
 Modified:coyote/src/java/org/apache/coyote Response.java
 Log:
 Fixed Bugtraq 6152759 (Default charset not included in Content-Type
 response header if no char encoding was specified).
 According to the Servlet 2.4 spec, calling:
   ServletResponse.setContentType(text/html);
 must yield these results:
   ServletResponse.getContentType() - text/html
   Content-Type response header - text/html;charset=ISO-8859-1
 Notice the absence of a charset in the result of getContentType(), but
 its presence (set to the default ISO-8859-1) in the Content-Type
 response header.
 Tomcat is currently not including the default charset in the
 Content-Type response header if no char encoding was specified.

-1.  This gets us right back to the same old problem where we are sending
back image/gif; charset=iso-8859-1, and nobody can read the response.
yes, sorry, I had forgotten about that case.
If we're not going to assume that the UA believes that the default encoding
is iso-8859-1 (which is what we are doing now),
I think the reason the spec added the requirement to clearly identify
the encoding in all cases (when using a writer) was because many
browsers let the user choose
which encoding to apply to responses that don't declare their encoding,
which will result in data corruption if the response was encoded in
ISO-8859-1 and the user picks an incompatible encoding.
then I'd suggest simply
doing:
   setCharacterEncoding(getCharacterEncoding());
in Response.getWriter (since the spec only requires that we identify the
charset when using a Writer, and we don't really know what it is when using
OutputStream).
The problem with this is that if you call getWriter() (with your 
proposed fix) followed by getContentType(), the returned content type
will include a charset, which is against the spec of getContentType():

  * If no character encoding has been specified, the
  * charset parameter is omitted.
This is why we need to append the default charset to the value of the
Content-Type header, if no char encoding has been specified.
Jan



This message is intended only for the use of the person(s) listed above as the 
intended recipient(s), and may contain information that is PRIVILEGED and 
CONFIDENTIAL.  If you are not an intended recipient, you may not read, copy, or 
distribute this message or any attachment. If you received this communication in 
error, please notify us immediately by e-mail and then delete all copies of this 
message and any attachments.
In addition you should be aware that ordinary (unencrypted) e-mail sent through the 
Internet is not secure. Do not send confidential or sensitive information, such as 
social security numbers, account numbers, personal identification numbers and 
passwords, to us via ordinary (unencrypted) e-mail.


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11 Http11Processor.java

2004-07-28 Thread Jan Luehe
Remy,
Remy Maucherat wrote:
Remy Maucherat wrote:
Cool. So after all the efforts I'm doing to optimize, you casually add 
GC, because the servlet API is completely stupid ?
So -1 for your patch: you need to rework it. I also didn't read all 
that funny stuff in the specification, so where does it come from ? ;)

I thought about it some more, and in addition to the performance 
problem, it would lead to adding charset even for binaries. We already 
found out that this way very bad (in addition to being meaningless, it 
breaks some clients such as Acrobat which simply does a check on the 
MIME type without removing the charset, which is rather logical since 
they're dealing with binaries) - this was some unintended behavior in 
some 4.1.x release (if I remember well, it was introduced by you 
already: you do like charset related issues ;) ).
yes, sorry, I had forgotten about that case.
Believe, I didn't commit yesterday's patch simply because
I don't have anything better to do. It's come up as a compliance
issue which had been masked by an unrelated problem that was fixed.
So what I suggest is that you send that you send that as feedback to the 
servlet API, and that they put a small errata for the specification.
-1 for implementing the requirements of the specificatrion (although I 
can't find them anywhere ;) ) to the letter right now, as they are 
broken (I didn't dislike the previous behavior, so I want it to remain).
ServletResponse.setCharacterEncoding/setContentType/setLocale...
* pContainers must communicate the character encoding used for
* the servlet response's writer to the client if the protocol
* provides a way for doing so. In the case of HTTP, the character
* encoding is communicated as part of the codeContent-Type/code
* header for text media types. Note that the character encoding
* cannot be communicated via HTTP headers if the servlet does not
* specify a content type; however, it is still used to encode text
* written via the servlet response's writer.
Reason: Many browsers let the user choose which encoding to apply to
responses that don't declare their encoding, which will result in data
corruption if the response was encoded in ISO-8859-1 and the user
picks an incompatible encoding.
ServletResponse.getContentType...:
* If no character encoding has been specified, the
* charset parameter is omitted.
Reason: Allow setLocale() to set the response's char encoding
(corresponding to the locale) if the char encoding has not already
been set by setContentType() or setCharacterEncoding().
You can find corresponding wordings in the servlet spec.
Therefore, we need to add a charset to the Content-Type response header if:
- no response encoding has been specified (ie., the return value of
  ServletResponse.getContentType() has no charset), and
- the response is using a writer
My patch did not check for the latter.
Jan
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-connectors/coyote/src/java/org/apache/coyote Response.java

2004-07-28 Thread Jan Luehe
Remy,
Remy Maucherat wrote:
Jan Luehe wrote:
Bill,
then I'd suggest simply
doing:
   setCharacterEncoding(getCharacterEncoding());
in Response.getWriter (since the spec only requires that we identify the
charset when using a Writer, and we don't really know what it is when 
using
OutputStream).

The problem with this is that if you call getWriter() (with your 
proposed fix) followed by getContentType(), the returned content type
will include a charset, which is against the spec of getContentType():

  * If no character encoding has been specified, the
  * charset parameter is omitted.
This is why we need to append the default charset to the value of the
Content-Type header, if no char encoding has been specified.

This is not acceptable, and is not an option, so you shouldn't be using 
we need, because we won't ;)
This is why I said we need instead of we will. ;-)
I agreed with Bill's proposed solution in principle (include charset
only when using writer), but pointed out that his proposed patch
would break ServletResponse.getContentType(), which is why I said that
if we were to include the default charset (if no charset was specified
and if a writer was being used), we'd have to do it at the time of
generating the header.
I'll reply to your other mail shortly.
Jan

The right solution is IMO to point out the issues to the specification 
people.

Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-connectors/coyote/src/java/org/apache/coyote Response.java

2004-07-28 Thread Jan Luehe
Jan Luehe wrote:
Remy,
Remy Maucherat wrote:
Jan Luehe wrote:
Bill,
then I'd suggest simply
doing:
   setCharacterEncoding(getCharacterEncoding());
in Response.getWriter (since the spec only requires that we identify 
the
charset when using a Writer, and we don't really know what it is 
when using
OutputStream).


The problem with this is that if you call getWriter() (with your 
proposed fix) followed by getContentType(), the returned content type
will include a charset, which is against the spec of getContentType():

  * If no character encoding has been specified, the
  * charset parameter is omitted.
This is why we need to append the default charset to the value of the
Content-Type header, if no char encoding has been specified.

This is not acceptable, and is not an option, so you shouldn't be 
using we need, because we won't ;)

This is why I said we need instead of we will. ;-)
I agreed with Bill's proposed solution in principle (include charset
only when using writer), but pointed out that his proposed patch
would break ServletResponse.getContentType(), which is why I said that
if we were to include the default charset (if no charset was specified
and if a writer was being used), we'd have to do it at the time of
generating the header.
I'll reply to your other mail shortly.
Actually, I just found that Bill's patch would fix the issue and
be compliant with the spec:
ServletResponse.getWriter:
 * If the response's character encoding has not been
 * specified as described in codegetCharacterEncoding/code
 * (i.e., the method just returns the default value
 * codeISO-8859-1/code), codegetWriter/code
 * updates it to codeISO-8859-1/code.
So it *is* expected for getContentType() to include a
charset=ISO-8859-1 if no char encoding had been specified
before getWriter() was called.
This way, the charset would automatically be included in the
Content-Type response header.
I'll revert my patch and apply Bill's solution.
Thanks, Bill!
Jan

Jan

The right solution is IMO to point out the issues to the specification 
people.

Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


[PATCH] jakarta-servletapi-5: javax/servlet/http/HttpServlet.java

2004-06-07 Thread Jan Luehe
This fixes an ArrayIndexOutOfBoundsException when superclass does
not declare any methods (see Bugtraq 4968841).
Jan
[EMAIL PROTECTED]'s password: 
Warning: Remote host denied X11 forwarding, perhaps xauth program could not be run on 
the server side.
Index: jsr154/src/share/javax/servlet/http/HttpServlet.java
===
RCS file: 
/home/cvs/jakarta-servletapi-5/jsr154/src/share/javax/servlet/http/HttpServlet.java,v
retrieving revision 1.7
diff -u -r1.7 HttpServlet.java
--- jsr154/src/share/javax/servlet/http/HttpServlet.java18 Mar 2004 16:40:35 
-  1.7
+++ jsr154/src/share/javax/servlet/http/HttpServlet.java8 Jun 2004 00:14:28 
-
@@ -465,36 +465,28 @@
 }
 
 
+private static Method[] getAllDeclaredMethods(Class c) {
 
+if (c.equals(javax.servlet.http.HttpServlet.class)) {
+return null;
+}
 
-
-private Method[] getAllDeclaredMethods(Class c) {
-   if (c.getName().equals(javax.servlet.http.HttpServlet))
-   return null;
-   
-   int j=0;
-   Method[] parentMethods = getAllDeclaredMethods(c.getSuperclass());
-   Method[] thisMethods = c.getDeclaredMethods();
+Method[] parentMethods = getAllDeclaredMethods(c.getSuperclass());
+Method[] thisMethods = c.getDeclaredMethods();

-   if (parentMethods!=null) {
-   Method[] allMethods =
-   new Method[parentMethods.length + thisMethods.length];
-   for (int i=0; iparentMethods.length; i++) {
-   allMethods[i]=parentMethods[i];
-   j=i;
-   }
-   j++;
-   for (int i=j; ithisMethods.length+j; i++) {
-   allMethods[i] = thisMethods[i-j];
-   }
-   return allMethods;
+if ((parentMethods != null)  (parentMethods.length  0)) {
+Method[] allMethods =
+new Method[parentMethods.length + thisMethods.length];
+   System.arraycopy(parentMethods, 0, allMethods, 0,
+ parentMethods.length);
+   System.arraycopy(thisMethods, 0, allMethods, parentMethods.length,
+ thisMethods.length);
+
+   thisMethods = allMethods;
}
+
return thisMethods;
 }
-
-
-
-
 
 
 /**

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Request to have log message for TagFileProcessor changed

2004-05-12 Thread Jan Luehe
I accidentally committed a wrong log message with my latest commit for
org.apache.jasper.compiler.TagFileProcessor.
Could someone with cvsadmin privileges change the log message for
the head version (revision 1.59) of this file as follows:
cvs admin -m 1.59:Fixed Bugzilla 28937: No error message for 
jsp.variable.alias TagFileProcessor.java

Thanks,

Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: New RM for Tomcat 5

2004-05-10 Thread Jan Luehe
+1

Jan

Remy Maucherat wrote:
Hi,

Yoav has expressed interest in being the release manager for Tomcat 5. 
Since he has shown interest in nearly all Tomcat components, and 
apparently has enough time at the moment, I think he would be the most 
qualified to replace me, and has my +1.

Rémy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina Manager.java

2004-04-22 Thread Jan Luehe
Hi Remy,

Remy Maucherat wrote:
[EMAIL PROTECTED] wrote:

luehe   2004/04/21 16:37:07

  Modified:catalina/src/share/org/apache/catalina Manager.java
  Log:
  Expose more of the session management methods at the top-level 
Manager interface


This is not a good idea since:
- it changes the API (ok, it likely won't break any existing impl)
- AFAIK, the basic manager doesn't care about session expiration
the reason I wanted to expose these methods at the Manager level
is that it makes using various session manager implementations more
seamless, without having to cast to specific impl classes.
Also, this will force future manager impls to use consistent
method signatures and names (ie., use getExpiredSessions() vs.
getExpired() or something like that).
You're right that the basic manager does not care about expired
sessions, but that's why it is abstract.
If a particular manager impl does not care about expired sessions,
they can simply return zero. ;-)
Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: jasper compiler problem semi-ready patch

2004-04-20 Thread Jan Luehe
Martin,

thanks for reporting the issue, and proposing a patch.

Your patch is consistent with this recommendation in the class comment
of java.net.URLConnection:
  Calling the ttclose()/tt methods on the ttInputStream/tt or
  ttOutputStream/tt of an ttURLConnection/tt after a request may
  free network resources associated with this instance, unless
  particular protocol specifications specify different behaviours for
  it.
I've applied your patch verbatim.

Jan

Martin Roos wrote:
as i read the site docs this is the correct list where to post the issue
to make the story short : jasper leaves filehandles open as it's compiling
any jsp. the bug is in the
org/apache/jasper/compiler/Compiler.java named file
around the lines ~500
function called public boolean isOutDated(boolean checkClass)
(i really have no time to play around with the cvs,  have a lot of work 
to do,
hey but at least i found out where the problem resides. so all this 
information here
is from jakarta-tomcat-5.0.19-src)

the thing is that the code there does the following action to check if 
the src of
the jsp is up to date (in this case asking for it's last modified time) :

---
jspRealLastModified = jspUrl.openConnection().getLastModified();
---
you see the problem is that the jspUrl.openConnection() opens a stream
to the file when getLastModified is being asked, but it never gets closed.
as tomcat seems to call garbage collect right after this compilation the
garbage-collector manages to clean it up. but if the garbage collector
isn't called for a while (as an example in jetty server) the file 
handles stay
open althrough they really aren't used for anything besided the 
getLastModified() call.

now when i fixed it to this

---
java.net.URLConnection uc = jspUrl.openConnection();
jspRealLastModified = uc.getLastModified();
uc.getInputStream().close();
--
it closes up nicely and no forgotten streams are left in the jvm, also
it doesn't need a garbage collect after it to clean the filehandle up.
i hope someone who has write access to the jasper source can fix it,
i'm not sure that my code written here should be the one to use, but
at least it works.
Martin

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: pageEncoing and contentType

2004-04-19 Thread Jan Luehe
seiji takegata wrote:
Hi,

I'm trying to generate PDF document from JSP, using itext library.
 (http://www.lowagie.com/iText/)  

I set contentType attribute to get browser open AdobeReader, and
pageEncoding to get right encoding for Japanese characters,
%@ page contentType=application/pdf pageEncoding=Shift_JIS %

Jasper translates this line to;

response.setContentType(application/pdf;charset=Shift_JIS);

I want to set response type to look like PDF without any 
particular encoding, and to tell Jasper that my source 
encoded with Shift_JIS, because IE does not recognize 
the contentType if charset option is added.

My question is:
1. Why Jasper adds charset option when I specify pageEncoding attribute?
2. Can I make Jasper not to add charset option when I use pageEncoding?
I posted same question to tomcat-user ML, then I was suggested to post 
this question to tomcat-dev list.

This is my first message to this list.
Notice that this is consistent with the JSP 2.0 spec (see JSP.4.2:
Response Character Encoding):
  The initial response char encoding is set to the CHARSET value of
  the contentType attribute of the page directive. If the page doesn't
  provide this attribute or the attribute does not have a CHARSET value
  [as in your case], the initial response char encoding is determined as
  follows:
  * ...
  * For JSP pages in standard syntax, it is the character encoding 
specified
by the pageEncoding attribute of the page directive ...

The reason behind this is that if the page specifies a page
character encoding (other than ISO-8859-1), it probably contains
non-Latin characters that may get lost if the response switched to
another char encoding.
If this is not what you want, you should consider using XML syntax for
your JSPs, in which case the response char encoding is set to UTF-8
(under the above circumstances), regardless of the autodetected page
source encoding.
Jan




Thanks you.
--
seiji takegata
[EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/valves RequestFilterValve.java

2004-04-14 Thread Jan Luehe
Sandy McArthur wrote:
Does this mean J2SE 1.3 support is no more?

On Apr 14, 2004, at 1:45 PM, [EMAIL PROTECTED] wrote:

  Log:
  Added support for exception chaining.
  +iae.initCause(e);
If there is a strong desire to maintain BC with J2SE 1.3,
I'll resort to the JdkCompat mechanism, but both Remy and Jess
voiced (strongly in the case of Jess) opinions that there was no need
for it.
Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/StandardContext.java

2004-04-07 Thread Jan Luehe
[For some reason, a commit notification was never sent for this
change i committed last night]
date: 2004/04/07 02:27:47;  author: luehe;  state: Exp;  lines: +18 -18
Fixed problem where when replacing global JspServlet with
webapp-specific one, the wrapper for the global JspServlet (which had
already been stopped and marked unavailable) was still being
registered with JMX, because even though the wrapper had been removed
from the context's children, it was never removed from the wrappers
instance field.
Replaced all occurrences of wrappers with the result of
findChildren(), so that we don't need to maintain two lists of
wrapper children.
DIFFS:

Index: StandardContext.java
===
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/StandardContext.java,v
retrieving revision 1.123
retrieving revision 1.124
diff -u -r1.123 -r1.124
--- StandardContext.java5 Apr 2004 22:40:07 -   1.123
+++ StandardContext.java7 Apr 2004 02:27:47 -   1.124
@@ -109,7 +109,7 @@
  *
  * @author Craig R. McClanahan
  * @author Remy Maucherat
- * @version $Revision: 1.123 $ $Date: 2004/04/05 22:40:07 $
+ * @version $Revision: 1.124 $ $Date: 2004/04/07 02:27:47 $
  */

 public class StandardContext
@@ -569,8 +569,6 @@
  */
 private transient DirContext webappResources = null;
-private ArrayList wrappers=new ArrayList();
-
 private long startupTime;
 private long startTime;
 private long tldScanTime;
@@ -2315,7 +2313,6 @@
 public Wrapper createWrapper() {
 //log.info( Create wrapper );
 Wrapper wrapper = new StandardWrapper();
-wrappers.add(wrapper);
 synchronized (instanceListeners) {
 for (int i = 0; i  instanceListeners.length; i++) {
@@ -3105,9 +3102,10 @@
  */
 public void removeChild(Container child) {
-if (!(child instanceof Wrapper))
+if (!(child instanceof Wrapper)) {
 throw new IllegalArgumentException
 (sm.getString(standardContext.notWrapper));
+}
 super.removeChild(child);

@@ -4416,8 +4414,6 @@

 // Reset application context
 context = null;
-
-wrappers = new ArrayList();
 }
 /**
@@ -4523,8 +4519,6 @@
 // Reset application context
 context = null;
-wrappers = new ArrayList();
-
 // This object will no longer be visible or used.
 try {
 resetContext();
@@ -5251,16 +5245,23 @@
 }
-/** JSR77 servlets attribute
+/**
+ * JSR77 servlets attribute
  *
  * @return list of all servlets ( we know about )
  */
 public String[] getServlets() {
-int size=wrappers.size();
-String result[]=new String[size];
-for( int i=0; i size; i++ ) {
-result[i]=((StandardWrapper)wrappers.get(i)).getObjectName();
+
+String[] result = null;
+
+Container[] children = findChildren();
+if (children != null) {
+result = new String[children.length];
+for( int i=0; i children.length; i++ ) {
+result[i] = ((StandardWrapper)children[i]).getObjectName();
+}
 }
+
 return result;
 }
@@ -5325,10 +5326,9 @@
 broadcaster.sendNotification(notification);
 }
 }
-for (Iterator it = wrappers.iterator(); it.hasNext() ; ) {
-StandardWrapper wrapper=(StandardWrapper)it.next();
-// XXX prevent duplicated registration
-wrapper.registerJMX( this );
+Container children[] = findChildren();
+for (int i=0; children!=null  ichildren.length; i++) {
+((StandardWrapper)children[i]).registerJMX( this );
 }
 } catch (Exception ex) {
 log.info(Error registering wrapper with jmx  + this +   +
Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core StandardContext.java

2004-04-07 Thread Jan Luehe
Remy Maucherat wrote:
[EMAIL PROTECTED] wrote:

luehe   2004/04/07 14:34:12

  Modified:catalina/src/share/org/apache/catalina/core
StandardContext.java
  Log:
  When the webapp specific JspServlet inherits the mappings from the
  global JspServlet, we need to wipe out the wrapper corresponding to
  the global JspServlet from the mapper (by calling
  StandardContext.addServletMapping() instead of
  StandardWrapper.addMapping())


Are you absolutely certain all this stuff is really needed ?
(I hope you're aware that adding a lot of these hacks is bad ;) )
Yes, I'm pretty positive, because I have some test cases. ;-)

The latest change was prompted by a test case where inside
a webapp that declares its own JspServlet, a JSP
jsp:include's another JSP. However, the ApplicationDispatcher
reported that the Servlet jsp is currently unavailable, because
the context's mapper still contained the wrapper corresponding to the
old (global) JspServlet, which had been disabled.
I promise I'm done. :)

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session StandardSession.java

2004-03-11 Thread Jan Luehe
Remy Maucherat wrote:
Remy Maucherat wrote:

[EMAIL PROTECTED] wrote:

luehe   2004/03/10 20:18:31

  Modified:catalina/src/share/org/apache/catalina/session
StandardSession.java
  Log:
  Fixed regression re: Bugtraq 4839736
  (HttpSession.setMaxInactiveInterval() doesn't behave as expected),
  which had been originally fixed in revision 1.20


-1. I don't agree with this change: the session is (apparently) 
supposed to be valid while it is being accessed, regardless of 
MaxInactiveInterval.


See bug 26051.
Your example from revision 1.20 seems invalid.
Yes, you're right, and I'm going to revert the change.

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: Question about HttpServletRequest.getParameterValues()

2004-03-02 Thread Jan Luehe
Remy Maucherat wrote:
Jan Luehe wrote:

This is a bug. The String[] returned by req.getParameterValues() should
have been a clone.
I just committed a fix.


I'd like to point out that this bug is not worth any performance drop. 
You should move those clones to the case where there's a security manager.
Fair enough.

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: Question about HttpServletRequest.getParameterValues()

2004-03-01 Thread Jan Luehe
Hi Christian,

The 2.3 HttpServletRequest interface provides a setAttribute() method to
change the values of a given attribute. It does NOT however provide a
similar setParameter() method, allowing you to programatically modify the
values that accompany the request - I assume this means that we shouldn't be
able to change these values.
What I've discovered however, is that if I _can_ modify parameter values by
calling getParameterValues() (which returns String[]) and set the values
that way. For instance:
Enumeration enum = req.getParameterNames();
while (enum.hasMoreElements()) {
String key =(String) enum.nextElement();
String vals[] = req.getParameterValues(key);
for (int i=0, max=vals.length; imax; i++) {
if (key.equalsIgnoreCase(password)) vals[i] = ;
logger.info(...key:+key+ value:+vals[i]);
}
}
This has the surprising (to me anyway) effect of actually _modifying_ the
underlying value for the particular key. Is this simply an implementation
oversight? I had assumed that the method would be returning a copy of the
underlying data structure, rather than a reference to the structure itself.
This isn't really a problem for me, but I thought it was interesting and I'm
curious to know if this was intentional or not. Anyone care to comment?
This is a bug. The String[] returned by req.getParameterValues() should
have been a clone.
I just committed a fix.

Thanks,

Jan


Thanks much,
Christian
--
Christian Cryder
Internet Architect, ATMReports.com
Project Chair, BarracudaMVC - http://barracudamvc.org
--
Coffee? I could quit anytime, just not today
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


[PROPOSAL] Configurable session id length

2004-02-06 Thread Jan Luehe
Is there any interest in making the session id length configurable?
If so, please consider my patch (attached).
Thanks,

Jan

Index: Manager.java
===
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java,v
retrieving revision 1.6
diff -u -r1.6 Manager.java
--- Manager.java13 Jan 2004 01:39:36 -  1.6
+++ Manager.java7 Feb 2004 02:19:31 -
@@ -164,6 +164,24 @@
 public void setMaxInactiveInterval(int interval);
 
 
+/**
+ * Gets the session id length (in bytes) of Sessions created by
+ * this Manager.
+ *
+ * @return The session id length
+ */
+public int getSessionIdLength();
+
+
+/**
+ * Sets the session id length (in bytes) for Sessions created by this
+ * Manager.
+ *
+ * @param sessionIdLength The session id length
+ */
+public void setSessionIdLength(int idLength);
+
+
 // - Public Methods
 
 
Index: session/ManagerBase.java
===
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/ManagerBase.java,v
retrieving revision 1.24
diff -u -r1.24 ManagerBase.java
--- session/ManagerBase.java26 Jan 2004 20:19:11 -  1.24
+++ session/ManagerBase.java7 Feb 2004 02:19:31 -
@@ -119,13 +119,6 @@
 
 
 /**
- * The number of random bytes to include when generating a
- * session identifier.
- */
-protected static final int SESSION_ID_BYTES = 16;
-
-
-/**
  * The message digest algorithm to be used when generating session
  * identifiers.  This must be an algorithm supported by the
  * codejava.security.MessageDigest/code class on your platform.
@@ -187,6 +180,12 @@
 
 
 /**
+ * The session id length of Sessions created by this Manager.
+ */
+protected int sessionIdLength = 16;
+
+
+/**
  * The descriptive name of this Manager implementation (for logging).
  */
 protected static String name = ManagerBase;
@@ -488,6 +487,36 @@
 
 
 /**
+ * Gets the session id length (in bytes) of Sessions created by
+ * this Manager.
+ *
+ * @return The session id length
+ */
+public int getSessionIdLength() {
+
+return (this.sessionIdLength);
+
+}
+
+
+/**
+ * Sets the session id length (in bytes) for Sessions created by this
+ * Manager.
+ *
+ * @param sessionIdLength The session id length
+ */
+public void setSessionIdLength(int idLength) {
+
+int oldSessionIdLength = this.sessionIdLength;
+this.sessionIdLength = idLength;
+support.firePropertyChange(sessionIdLength,
+   new Integer(oldSessionIdLength),
+   new Integer(this.sessionIdLength));
+
+}
+
+
+/**
  * Return the descriptive short name of this Manager implementation.
  */
 public String getName() {
@@ -496,8 +525,9 @@
 
 }
 
-/** Use /dev/random-type special device. This is new code, but may reduce the
- *  big delay in generating the random.
+/** 
+ * Use /dev/random-type special device. This is new code, but may reduce
+ * the big delay in generating the random.
  *
  *  You must specify a path to a random generator file. Use /dev/urandom
  *  for linux ( or similar ) systems. Use /dev/random for maximum security
@@ -828,23 +858,30 @@
  * Generate and return a new session identifier.
  */
 protected synchronized String generateSessionId() {
-byte bytes[] = new byte[SESSION_ID_BYTES];
-getRandomBytes( bytes );
-bytes = getDigest().digest(bytes);
+
+byte random[] = new byte[16];
 
 // Render the result as a String of hexadecimal digits
 StringBuffer result = new StringBuffer();
-for (int i = 0; i  bytes.length; i++) {
-byte b1 = (byte) ((bytes[i]  0xf0)  4);
-byte b2 = (byte) (bytes[i]  0x0f);
-if (b1  10)
-result.append((char) ('0' + b1));
-else
-result.append((char) ('A' + (b1 - 10)));
-if (b2  10)
-result.append((char) ('0' + b2));
-else
-result.append((char) ('A' + (b2 - 10)));
+int resultLenBytes = 0;
+while (resultLenBytes  this.sessionIdLength) {
+getRandomBytes(random);
+random = getDigest().digest(random);
+for (int j = 0;
+j  random.length  resultLenBytes  this.sessionIdLength;
+j++) {
+byte b1 = (byte) ((random[j]  0xf0)  4);
+byte b2 = (byte) (random[j]  0x0f);
+if (b1  10)
+result.append((char) ('0' + b1));
+else
+

Re: cvs commit: jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler TagLibraryInfoImpl.java

2004-02-03 Thread Jan Luehe
[EMAIL PROTECTED] wrote:
luehe   2004/02/03 12:20:19

  Modified:jasper2/src/share/org/apache/jasper/compiler
TagLibraryInfoImpl.java
  Log:
  Convert selected tag attribute types to their Fully-Qualified-Name
  equivalents if the taglib is JSP 1.2 based.
  
  This has been discussed with Mark Roth, the JSP spec lead, and will
  allow JSP 1.2 taglibs to continue to work on Tomcat 5. Notice that JSP
  1.2 did not require that tag attribute types be specified using their
  fully-qualified class names. This restriction was added in Tomcat 5.
I meant: JSP 2.0.

Jan


  
  Revision  ChangesPath
  1.52  +22 -7 jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/TagLibraryInfoImpl.java
  
  Index: TagLibraryInfoImpl.java
  ===
  RCS file: /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/TagLibraryInfoImpl.java,v
  retrieving revision 1.51
  retrieving revision 1.52
  diff -u -r1.51 -r1.52
  --- TagLibraryInfoImpl.java	7 Jan 2004 00:37:47 -	1.51
  +++ TagLibraryInfoImpl.java	3 Feb 2004 20:20:19 -	1.52
  @@ -288,7 +288,7 @@
   else if (validator.equals(tname))
   this.tagLibraryValidator = createValidator(element);
   else if (tag.equals(tname))
  -tagVector.addElement(createTagInfo(element));
  +tagVector.addElement(createTagInfo(element, jspversion));
   else if (tag-file.equals(tname)) {
   TagFileInfo tagFileInfo = createTagFileInfo(element, uri,
   jarFileUrl);
  @@ -382,7 +382,9 @@
   return location;
   }
   
  -private TagInfo createTagInfo(TreeNode elem) throws JasperException {
  +private TagInfo createTagInfo(TreeNode elem, String jspVersion)
  +throws JasperException {
  +
   String tagName = null;
   String tagClassName = null;
   String teiClassName = null;
  @@ -430,7 +432,7 @@
   } else if (variable.equals(tname)) {
   variableVector.addElement(createVariable(element));
   } else if (attribute.equals(tname)) {
  -attributeVector.addElement(createAttribute(element));
  +attributeVector.addElement(createAttribute(element, jspVersion));
   } else if (dynamic-attributes.equals(tname)) {
   dynamicAttributes = JspUtil.booleanValue(element.getBody());
   } else if (example.equals(tname)) {
  @@ -526,7 +528,7 @@
   return new TagFileInfo(name, path, tagInfo);
   }
   
  -TagAttributeInfo createAttribute(TreeNode elem) {
  +TagAttributeInfo createAttribute(TreeNode elem, String jspVersion) {
   String name = null;
   String type = null;
   boolean required = false, rtexprvalue = false, reqTime = false,
  @@ -549,6 +551,19 @@
   rtexprvalue = JspUtil.booleanValue(s);
   } else if (type.equals(tname)) {
   type = element.getBody();
  +if (1.2.equals(jspVersion)
  + (type.equals(Boolean)
  +|| type.equals(Byte)
  +|| type.equals(Character)
  +|| type.equals(Double)
  +|| type.equals(Float)
  +|| type.equals(Integer)
  +|| type.equals(Long)
  +|| type.equals(Object)
  +|| type.equals(Short)
  +|| type.equals(String))) {
  +type = java.lang. + type;
  +}
   } else if (fragment.equals(tname)) {
   String s = element.getBody();
   if (s != null)
  
  
  

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler TagLibraryInfoImpl.java

2004-02-03 Thread Jan Luehe
Jan Luehe wrote:
[EMAIL PROTECTED] wrote:

luehe   2004/02/03 12:20:19

  Modified:jasper2/src/share/org/apache/jasper/compiler
TagLibraryInfoImpl.java
  Log:
  Convert selected tag attribute types to their Fully-Qualified-Name
  equivalents if the taglib is JSP 1.2 based.
This has been discussed with Mark Roth, the JSP spec lead, and will
  allow JSP 1.2 taglibs to continue to work on Tomcat 5. Notice that JSP
  1.2 did not require that tag attribute types be specified using their
  fully-qualified class names. This restriction was added in Tomcat 5.


I meant: JSP 2.0.
Just to be clear: I meant the restriction was added to JSP 2.0
(and implemented by Tomcat 5, of course).
Sorry if there was any confusion. :)

Jan



Jan


Revision  ChangesPath
  1.52  +22 -7 
jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/TagLibraryInfoImpl.java 

Index: TagLibraryInfoImpl.java
  ===
  RCS file: 
/home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/TagLibraryInfoImpl.java,v 

  retrieving revision 1.51
  retrieving revision 1.52
  diff -u -r1.51 -r1.52
  --- TagLibraryInfoImpl.java7 Jan 2004 00:37:47 -1.51
  +++ TagLibraryInfoImpl.java3 Feb 2004 20:20:19 -1.52
  @@ -288,7 +288,7 @@
   else if (validator.equals(tname))
   this.tagLibraryValidator = createValidator(element);
   else if (tag.equals(tname))
  -tagVector.addElement(createTagInfo(element));
  +tagVector.addElement(createTagInfo(element, 
jspversion));
   else if (tag-file.equals(tname)) {
   TagFileInfo tagFileInfo = 
createTagFileInfo(element, uri,
   
jarFileUrl);
  @@ -382,7 +382,9 @@
   return location;
   }
 -private TagInfo createTagInfo(TreeNode elem) throws 
JasperException {
  +private TagInfo createTagInfo(TreeNode elem, String jspVersion)
  +throws JasperException {
  +
   String tagName = null;
   String tagClassName = null;
   String teiClassName = null;
  @@ -430,7 +432,7 @@
   } else if (variable.equals(tname)) {
   variableVector.addElement(createVariable(element));
   } else if (attribute.equals(tname)) {
  -attributeVector.addElement(createAttribute(element));
  +attributeVector.addElement(createAttribute(element, 
jspVersion));
   } else if (dynamic-attributes.equals(tname)) {
   dynamicAttributes = 
JspUtil.booleanValue(element.getBody());
   } else if (example.equals(tname)) {
  @@ -526,7 +528,7 @@
   return new TagFileInfo(name, path, tagInfo);
   }
 -TagAttributeInfo createAttribute(TreeNode elem) {
  +TagAttributeInfo createAttribute(TreeNode elem, String 
jspVersion) {
   String name = null;
   String type = null;
   boolean required = false, rtexprvalue = false, reqTime = 
false,
  @@ -549,6 +551,19 @@
   rtexprvalue = JspUtil.booleanValue(s);
   } else if (type.equals(tname)) {
   type = element.getBody();
  +if (1.2.equals(jspVersion)
  + (type.equals(Boolean)
  +|| type.equals(Byte)
  +|| type.equals(Character)
  +|| type.equals(Double)
  +|| type.equals(Float)
  +|| type.equals(Integer)
  +|| type.equals(Long)
  +|| type.equals(Object)
  +|| type.equals(Short)
  +|| type.equals(String))) {
  +type = java.lang. + type;
  +}
   } else if (fragment.equals(tname)) {
   String s = element.getBody();
   if (s != null)
 
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5 CoyoteRequest.java

2003-11-20 Thread Jan Luehe
Remy Maucherat wrote:
Larry Isaacs wrote:

Hi Jan,

This looked like a good idea, so I ported it to Tomcat 4.1.x.
I'll go ahead and un-port it for consistincy with Tomcat 5.


I thought it was a good idea too. Too bad this has to be unported.
I'll see what I can do to have this fix back in.
Maybe the failing compatibility tests can be excluded, until the spec
has been clarified.
Will let you know.

Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


[PATCH][jakarta-servletapi-5/jsr152] Upgrade to JSTL 1.1

2003-11-20 Thread Jan Luehe
Patch is attached.

Also, please update jstl.jar and standard.jar in

  jakarta-servletapi-5/jsr152/examples/WEB-INF/lib

with their JSTL-1.1 counterparts from

http://www.tux.org/pub/net/apache/dist/jakarta/taglibs/standard/binaries/

Thanks!

Jan
Executing ssh-askpass to query the password...
Warning: Remote host denied X11 forwarding, perhaps xauth program could not be run on 
the server side.
cvs server: Diffing jakarta-servletapi-5/jsr152
cvs server: Diffing jakarta-servletapi-5/jsr152/examples
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/cal
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/checkbox
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/colors
cvs server: Diffing 
jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/compressionFilters
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/dates
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/error
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/examples
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/filters
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/jsp2
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/jsp2/examples
cvs server: Diffing 
jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/jsp2/examples/el
cvs server: Diffing 
jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/jsp2/examples/simpletag
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/listeners
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/num
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/sessions
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/util
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/classes/validators
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/jsp
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/jsp/applet
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/jsp2
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/lib
Index: jakarta-servletapi-5/jsr152/examples/WEB-INF/lib/jstl.jar
===
RCS file: /home/cvs/jakarta-servletapi-5/jsr152/examples/WEB-INF/lib/jstl.jar,v
retrieving revision 1.1
diff -u -r1.1 jstl.jar
Binary files /tmp/cvsTsksTg and jstl.jar differ
Index: jakarta-servletapi-5/jsr152/examples/WEB-INF/lib/standard.jar
===
RCS file: /home/cvs/jakarta-servletapi-5/jsr152/examples/WEB-INF/lib/standard.jar,v
retrieving revision 1.1
diff -u -r1.1 standard.jar
Binary files /tmp/cvs5zNo7N and standard.jar differ
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/WEB-INF/tags
Index: jakarta-servletapi-5/jsr152/examples/WEB-INF/tags/displayProducts.tag
===
RCS file: 
/home/cvs/jakarta-servletapi-5/jsr152/examples/WEB-INF/tags/displayProducts.tag,v
retrieving revision 1.2
diff -u -r1.2 displayProducts.tag
--- jakarta-servletapi-5/jsr152/examples/WEB-INF/tags/displayProducts.tag   28 Oct 
2002 17:45:44 -  1.2
+++ jakarta-servletapi-5/jsr152/examples/WEB-INF/tags/displayProducts.tag   20 Nov 
2003 21:45:38 -
@@ -2,7 +2,7 @@
- Copyright (c) 2002 The Apache Software Foundation.  All rights 
- reserved.
 --%
-%@ taglib prefix=c uri=http://java.sun.com/jstl/core_rt; %
+%@ taglib prefix=c uri=http://java.sun.com/jsp/jstl/core; %
 %@ attribute name=normalPrice fragment=true %
 %@ attribute name=onSale fragment=true %
 %@ variable name-given=name %
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/cal
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/checkbox
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/colors
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/dates
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/error
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/forward
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/images
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/include
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/jsp2
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/jsp2/el
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/jsp2/jspattribute
cvs server: Diffing jakarta-servletapi-5/jsr152/examples/jsp2/jspx
Index: jakarta-servletapi-5/jsr152/examples/jsp2/jspx/basic.jspx
===
RCS file: /home/cvs/jakarta-servletapi-5/jsr152/examples/jsp2/jspx/basic.jspx,v
retrieving revision 1.3
diff -u -r1.3 basic.jspx
--- 

Re: [VOTE] New committer: Mark Thomas

2003-11-20 Thread Jan Luehe
+1.

Jan

Remy Maucherat wrote:
Hi,

I'd like to nominate Mark Thomas as a Tomcat committer. He has 
contibuted a significant amount of fixes already, and does what nobody 
else does: roam Bugzila to fix older issues and cleanup the database. He 
has special interest in the WebDAV code, which currently has no maintainer.

He has my +1.

Rémy



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: Connector ObjectName includes address

2003-11-05 Thread Jan Luehe

Bill Barker wrote:

Currently, connector objectname includes address in this format:

domain:type=Connector,port=8080,address=0.0.0.0.

This causes a problem when IPV6 addresses are used since IPV6 addresses
include colons.  The javax.management.ObjectName doesn't allow to have
colon character in it and this causes
javax.management.MalformedObjectNameException: ObjectName: invalid
character ':' in value part of property.
I propose to exclude address as connector objectname property since port
number alone should keep the objectnames unique.
What about the case where I have several V-Hosts, each with it's own
Connector listening on port 80?
This seems a possible use case, but is that really useful ? I would
always put the connector at the engine level, personally.


Regardless of where you put it, you still have one connector per V-Host IP
address.


I may be totally off here, but in the most common cases, all your
domain names (one for each V-Host) will map to the same IP address, in
which case you cannot have more than one Connector listening on the
same port (if you do that, you'll get a java.net.BindException when
starting the 2nd and so forth Connector).
You can only have more than one Connector listening on the same port
when they're listening on different IP addresses on the same server.
This is the case we need to resolve, right?

Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/net PoolTcpEndpoint.java

2003-10-31 Thread Jan Luehe
Remy Maucherat wrote:
Jan Luehe wrote:

Remy Maucherat wrote:
I guess I don't understand what makes 1 bad but 2 OK. Where do we 
draw the line of what is a stupid config?


Yes, definitely, 2 is nearly as stupid as 1. We need a reasonable 
minimal value for maxThreads; let's say 10 - 20.
Remy,

I agree we should help users come up with reasonable config values,
but I'm just afraid rejecting any maxThreads  10 or  20 will send the
wrong message, as if there was a bug in the way we dispatch incoming
requests that requires at least 10 threads.
I think we should make any maxThreads setting work, as my patch
attempts to do, and update the documentation to make users aware of
the limitations they will run into when picking a low maxThreads.
I think that would be the cleanest solution.
Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/net PoolTcpEndpoint.java

2003-10-31 Thread Jan Luehe
Remy,

I agree we should help users come up with reasonable config values,
but I'm just afraid rejecting any maxThreads  10 or  20 will send the
wrong message, as if there was a bug in the way we dispatch incoming
requests that requires at least 10 threads.


Nope, I disagree. If maxThreads  (say) 10, then set it to 10. Allowing 
broken settings is bad, as there will be people out there who will use 
them, and then will assume Tomcat is broken.
I think changing people's config values behind their backs is not such
a good idea in general.
I think we should make any maxThreads setting work, as my patch
attempts to do, and update the documentation to make users aware of
the limitations they will run into when picking a low maxThreads.
I think that would be the cleanest solution.


The rationale is that there's no point adding complexity and checks into 
the very critical algorithm, simply to be able to support broken cases.
I think we have 2 options:
1. Support any maxThreads setting (including 1, 2, etc.).
2. Reject any maxThreads values that are smaller than some
   threshold and throw an error.
The problem with 2. is that picking a value for the threshold (10? 20?) 
seems
rather arbitrary. Therefore, I think we should do 1. The complexity it 
is adding is not significant and is well-documented.

Please tell me you agree. :)

Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/net PoolTcpEndpoint.java

2003-10-30 Thread Jan Luehe
Remy Maucherat wrote:
Bill Barker wrote:

[EMAIL PROTECTED] wrote:


luehe   2003/10/30 13:01:39

 Modified:util/java/org/apache/tomcat/util/net PoolTcpEndpoint.java
 Log:
 Fixed problem where if maxThreads is set to 1,
 ThreadPool.findControlRunnable() will log this error on the first
 request:
   SEVERE: All threads (1) are currently busy, waiting. Increase
   maxThreads (1) or check the servlet status
 and then block forever


-1 for this patch.
1 is obviously a stupid configuration value, so the pool should 
refuse it.


I agree with Remy:  The place to check this is ThreadPool.


I'd like to add that my -1 is not because the patch is bad (the revised 
algorithm seems ok), but because the 1 value doesn't make sense, so I 
don't think there's a point adding a special case for it.
I guess I don't understand what makes 1 bad but 2 OK. Where do we 
draw the line of what is a stupid config?

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


[5.0] No sessions purged in Context with backgroundProcessorDelay 0?

2003-10-15 Thread Jan Luehe
I may be totally wrong here, but it seems that if the
backgroundProcessorDelay property on a StandardContext is set to
something greater than zero (default is -1, inherited from
ContainerBase), the context's sessions are never purged.
This is because in ContainerBase$ContainerBackgroundProcessor,
processChildren() is implemented as follows:
for (int i = 0; i  children.length; i++) {
if (children[i].getBackgroundProcessorDelay() = 0) {
processChildren(children[i], cl);
}
}
So when invoked from the ContainerBackgroundProcessor of a
StandardHost, only the sessions of those StandardContexts with a
backgroundProcessorDelay =0 will get purged.
I think the assumption is that if a StandardContext has a
backgroundProcessorDelay  0, it would have its own
ContainerBackgroundProcessor spawn at startup. However, unlike
StandardEngine/Host/Wrapper, StandardContext.start() does not invoke
super.start(), and therefore a ContainerBackgroundProcessor is never
created for a StandardContext.
Am I missing something here?

Thanks,

Jan





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [5.0] No sessions purged in Context with backgroundProcessorDelay 0?

2003-10-15 Thread Jan Luehe
Thanks for confirming, Remy!

I'll make these changes.

Jan

Remy Maucherat wrote:
Jan Luehe wrote:

I may be totally wrong here, but it seems that if the
backgroundProcessorDelay property on a StandardContext is set to
something greater than zero (default is -1, inherited from
ContainerBase), the context's sessions are never purged.
This is because in ContainerBase$ContainerBackgroundProcessor,
processChildren() is implemented as follows:
for (int i = 0; i  children.length; i++) {
if (children[i].getBackgroundProcessorDelay() = 0) {
processChildren(children[i], cl);
}
}
So when invoked from the ContainerBackgroundProcessor of a
StandardHost, only the sessions of those StandardContexts with a
backgroundProcessorDelay =0 will get purged.
I think the assumption is that if a StandardContext has a
backgroundProcessorDelay  0, it would have its own
ContainerBackgroundProcessor spawn at startup. However, unlike
StandardEngine/Host/Wrapper, StandardContext.start() does not invoke
super.start(), and therefore a ContainerBackgroundProcessor is never
created for a StandardContext.


Arg, stupid me, I forgot about that. We need to add the code which 
starts the backgroud thread in StandardContext.start.
So we need to call super.startThread() and super.stopThread. This 
doesn't seem too hard.

Remy



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


How to make CLIENT-CERT protection work?

2003-10-15 Thread Jan Luehe
Consider the following scenario:

1. Client sends POST request (with content type other than
   application/x-www-form-urlencoded) to SSL-enabled server (with
   client auth turned off).
2. Server parses request header, and determines that the resource
   identified by the request-URI is CLIENT-CERT protected.
3. Server's SSLAuthenticator valve reinitiates SSL handshake, w/
   client auth turned on.
4. The server sends its HelloRequest, and expects to read the client's
   ClientHello. However, what it gets is the POST request's body which
   hadn't been read yet.
5. SSL handshake fails.

In order to avoid this problem, SSLAuthenticator.authenticate()
clears the socket in the case of a POST request by reading the POST
request's body *before* reinitiating the handshake. To read the POST
body, it calls CoyoteRequest.getParameterMap(), which reads and
processes the POST body only if the content type equals
application/x-www-form-urlencoded.
Therefore, the SSL re-handshake works according to plan if the content
type equals application/x-www-form-urlencoded, but fails for any
other content type.
Should we always read the POST body in getParameterMap(), and cache it
in a byte[] if content type is different from
application/x-www-form-urlencoded, and have
CoyoteRequest.getInputStream()/getReader() return wrappers around this
byte[]?
Any better suggestions?

Thanks,

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-connectors/coyote/src/java/org/apache/coyote Response.java

2003-10-14 Thread Jan Luehe
Bill,

  this.contentType = contentType.substring(0, beginCharsetParam);
 -String tail = contentType.substring(beginCharsetParam + 1);
 +// Trim the semicolon preceding the charset
 +int charsetSemi = this.contentType.lastIndexOf(';');


This is still not working for me.  Now I can't do stuff like:
  setContentType(text/html;charset=utf-8;   altcharset=iso-latin-1);
I don't know any current browser that uses this, but it is completely valid
in RFC 2616.
can you describe how is this failing for you?
The above setContentType results in a content type setting of
  text/html;   altcharset=iso-latin-1

with the response charset set to utf-8.

I don't think I changed the previous behaviour in any way that would 
break things, but I might be wrong. :)

Let me know.

Jan



 +if (charsetSemi != -1) {
 +this.contentType = this.contentType.substring(0,
charsetSemi);

 +}
 +String tail = contentType.substring(beginCharsetParam);
  int nextParam = tail.indexOf(';');
  String charsetValue = null;
  if (nextParam != -1) {
  this.contentType += tail.substring(nextParam);
 -charsetValue = tail.substring(beginCharsetValue,
nextParam);

 +charsetValue = tail.substring(BEGIN_CHARSET_VALUE,
nextParam);

  } else {
 -charsetValue = tail.substring(beginCharsetValue);
 +charsetValue = tail.substring(BEGIN_CHARSET_VALUE);
  }
  // The charset value may be quoted, but must not contain any
quotes.

  charsetValue = charsetValue.replace('', ' ');




-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [5.0] Schedule change

2003-10-02 Thread Jan Luehe
Hans/Remy,

I don't know more than you do about when J2EE 1.4 will be released, but
the specs are starting to move through final approval now, so I'm pretty
sure it will happen in a month or two. Three months for running a few
Beta releases instead of releasing it as something it's not doesn't seem
to bad to me ...
Sun expects to ship J2EE 1.4 FCS sometime around the last week of November.

Hope this helps.

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/startup CatalinaProperties.java

2003-09-26 Thread Jan Luehe
Remy,

As I mentioned in my private email to you, it may not always be
practical to rely on the list of noTldJars configured in
catalina.properties if you bundle Tomcat with your own product. That's
why I added the setProperty method on CatalinaProperties, in order
to be able to override the value in catalina.properties. This allows
the list of noTldJars to be determined more accurately at
runtime. The code that is embedding Tomcat would call
CatalinaProperties.setProperty() *before* starting Tomcat.


And is exactly what I wanted to avoid. Again, CatalinaProperties is for 
configuring the bootstrap, not anything else (otherwise, stuff which 
belongs to server.xml will end up here, and it will end up being a huge 
mess).
I agree w/ that.

I leave the judgement up to the community.


I'll have to reiterate my -1.
OK, I'll revert my patch (and leave it as a private extension), until we 
have found a better solution that everybody agrees on.

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/startup CatalinaProperties.java

2003-09-26 Thread Jan Luehe
Remy Maucherat wrote:

OK, I'll revert my patch (and leave it as a private extension), until 
we have found a better solution that everybody agrees on.


Thanks :)

As for a solution, I believe your hardcoded list was acceptable, if not 
completely optimal.
OK, in this case, I'll add that part of the patch back in, without the 
CatalinaProperties.setProperty, which I'll keep as a private extension 
for now.

We can always revisit this issue at a later point.

I feel like I'm on a bazaar. :)

Thanks, Remy!

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/startup CatalinaProperties.java

2003-09-26 Thread Jan Luehe
Remy Maucherat wrote:
Jan Luehe wrote:

Remy Maucherat wrote:

As for a solution, I believe your hardcoded list was acceptable, if 
not completely optimal.


OK, in this case, I'll add that part of the patch back in, without the 
CatalinaProperties.setProperty, which I'll keep as a private extension 
for now.


I meant the hardcoded list that you put in TldConfig. I don't think 
CatalinaProperties should contain that kind of configuration options 
(which are meant to configure the Bootstrap class Tomcat standalone uses).
Got it.

Jan





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/startup CatalinaProperties.java

2003-09-26 Thread Jan Luehe
Remy Maucherat wrote:
Jan Luehe wrote:

Remy Maucherat wrote:

As for a solution, I believe your hardcoded list was acceptable, if 
not completely optimal.


OK, in this case, I'll add that part of the patch back in, without the 
CatalinaProperties.setProperty, which I'll keep as a private extension 
for now.


I meant the hardcoded list that you put in TldConfig. I don't think 
CatalinaProperties should contain that kind of configuration options 
(which are meant to configure the Bootstrap class Tomcat standalone uses).
Would you agree on a setNoTldJars method on TldConfig then, which 
would allow the hard-coded list to be overridden?

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/startup CatalinaProperties.java

2003-09-25 Thread Jan Luehe
Remy,

sorry, but I don't see which part of our email exchange you found
very frustrating. As I said, I'm open to suggestions, and if my
patch is deemed useless, I'll revert it. No problem.
I've done some measurements on my Ultra 60. TldConfig already
calculates the tldScanTime for each context, so that's the number I'm
using. Following are the tldScanTime numbers for a sequence of 10
startups with the standard contexts:
- w/o optimization (static initializer in TldConfig commented out):

Context name   tldScanTime for 10 runs   | 
Average
---
/manager   417/452/431/438/408/468/434/418/423/450   | 
433.90
/admin 1399/1310/1440/1385/1288/1347/1345/1340/1280/1317 
|1345.10
/servlets-examples 210/198/206/235/192/182/189/188/166/195   | 
196.10
[] 222/222/213/218/222/225/215/204/223/221   | 
218.50
/jsp-examples  996/841/854/862/900/855/903/845/856/815   | 
872.70
/tomcat-docs   155/176/168/184/175/294/164/185/194/183   | 
187.80

Total=3254.10

- With optimization

Context name   tldScanTime for 10 runs   | 
Average
---
/manager   239/225/242/375/230/241/234/228/237/319   | 
257.00
/admin 1225/1192/1242/1171/1320/1251/1248/1270/1253/1219 
|1239.10
/servlets-examples 74/69/165/79/92/71/106/96/95/123  | 
97.00
[] 129/109/136/186/241/194/111/103/96/142| 
144.70
/jsp-examples  728/736/757/843/812/796/782/753/820/813   | 
784.00
/tomcat-docs   77/60/127/191/102/101/61/96/88/85 | 
98.80

Total=2620.60

This may not be significant, but if you increase the number of
contexts and have additional JARs on your classpath of which you know
they don't contain any TLDs, we're talking seconds, not ms.
As I mentioned in my private email to you, it may not always be
practical to rely on the list of noTldJars configured in
catalina.properties if you bundle Tomcat with your own product. That's
why I added the setProperty method on CatalinaProperties, in order
to be able to override the value in catalina.properties. This allows
the list of noTldJars to be determined more accurately at
runtime. The code that is embedding Tomcat would call
CatalinaProperties.setProperty() *before* starting Tomcat.
I leave the judgement up to the community.

Thanks,

Jan

Remy Maucherat wrote:
[EMAIL PROTECTED] wrote:

luehe   2003/09/24 12:09:29

  Modified:catalina/src/share/org/apache/catalina/startup
CatalinaProperties.java
  Log:
  Added setProperty method.
For example, this will allow the value for the noTldJars 
property to
  be determined and set dynamically.


After a very frustrating private email exchange ...

As I said previously:

-1 for allowing configuration of a TLD exclusion list, because we're not 
out there to gain .01s on startup time, and a hardcoded list is good 
enough and will give us 99% of the benfit (which is already dubious IMO, 
but I don't mind about it overall). Also, (see below), the place where 
this is configured seems inappropriate.

-1 for allowing dynaminc configuration of CatalinaProperties. 
cataina.properties is only meant to be able to configure the Bootstrap 
class of Tomcat, and shouldn't be a place to hack in things we want to 
hide.

Please consider reverting the relevant patches. I think the best would 
be to completely remove the TLD exclusion list feature, as adding 
gratuitous complexity is bad.

Thanks :)

Remy



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/startup CatalinaProperties.java

2003-09-25 Thread Jan Luehe
Hans,

thanks for stepping in.

sorry, but I don't see which part of our email exchange you found
very frustrating. As I said, I'm open to suggestions, and if my
patch is deemed useless, I'll revert it. No problem.
[...]


Sorry for not jumping in earlier in this discussion.

When I implemented the shared tag libraries feature in LiteWebServer,
I added scanning of JAR files in just /shared/lib. I do _not_ scan JAR
files in common/lib and server/lib, because placing shared tag libraries
there would be point-less; the container doesn't need access to them,
only the applications do, which is what shared/lib is for.
Maybe that cuts down the list enough so you don't have to mess with
an exclusion list at all?
good point. What about environments that embed Tomcat without following
Tomcat's directory layout, and in which the classloader hierachy/depth 
is different from that in Tomcat?

Jan


Hans


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/startup CatalinaProperties.java

2003-09-25 Thread Jan Luehe
Hans,

good point. What about environments that embed Tomcat without following
Tomcat's directory layout, and in which the classloader hierachy/depth 
is different from that in Tomcat?


I don't give them that option (with regards to this) in LiteWebServer.
The only place I look for shared libraries is $CATALINA_HOME/shared/lib.
Is that really a severe limitation, you think?
I think it depends on whether you anticipate (or support) LiteWebServer 
being bundled (as one or more libraries) into other products that use a 
different directory layout, with different locations for shared libs. If 
that's the case, limiting the location of shared libs to 
$CATALINA_HOME/shared/lib might be too restrictive.

That's why Tomcat searches the webapp's classloader delegation chain for 
JARs, which ensures that it will find shared libs regardless of their 
location. Of course, by searching the classloader chain all the way up, 
we end up parsing too many JARs. I think it's impossible to know ahead 
of time where to stop in the chain, until you've actually traversed the 
entire chain. For example, an approach that walks the chain just two 
levels up won't work in environments where the classloaders 3 or 4 
levels up in the chain are also responsible for loading shared libs.

Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [PROPOSAL] Narrow down the list of JARs to be scanned for TLDs

2003-09-23 Thread Jan Luehe
Petr,

I haven't committed any changes related to the proposal yet, as I was 
waiting for more feedback. The changes I committed in TldConfig.java 
were unrelated.

As Jan has already pointed out, it should improve the startup time for
Tomcat 5 (since scanning TLD files is a major hit).
I too agree that limiting the number of jars scanned could substantially 
improve first start performance, however I am worried that the solution 
as suggested by Jan will make it too easy for the users to shoot 
themselves to the foot.
Actually, if they don't do anything, things will continue to work, as by 
default the container would continue to scan all globally shared JARs.
If they knew their webapp relied on just jstl.jar, they would configure 
the property with just that name.

Also, unless the user does some manual setup, 
there will be no performance gain,right? So wouldn't it be better to 
have a hardcoded list of well-known jar files that should be excluded 
from scanning? This would include all jars found in the Tomcat 
installation.
OK, I like that idea.

I also think that under no circumstances the jar files in WEB-INF/lib 
should be excluded from scanning, as that is in conflict with the spec.
That was never part of the proposal, the proposal only dealt with 
globally shared JARs, which represent a Tomcat-specific extension to the 
spec.

Thanks,

Jan


Or is there something I am missing about the proposal?

Petr

 

Remy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
  


 



This message is intended only for the use of the person(s) listed 
above as the intended recipient(s), and may contain information that 
is PRIVILEGED and CONFIDENTIAL.  If you are not an intended recipient, 
you may not read, copy, or distribute this message or any attachment. 
If you received this communication in error, please notify us 
immediately by e-mail and then delete all copies of this message and 
any attachments.

In addition you should be aware that ordinary (unencrypted) e-mail 
sent through the Internet is not secure. Do not send confidential or 
sensitive information, such as social security numbers, account 
numbers, personal identification numbers and passwords, to us via 
ordinary (unencrypted) e-mail.

 



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


[PROPOSAL] Narrow down the list of JARs to be scanned for TLDs

2003-09-22 Thread Jan Luehe
Currently, any JARs in the classloader delegation chain of a webapp's
classloader are scanned for packaged TLDs. This is convenient, as it
allows a JAR-packaged taglib to be simply dropped in common/lib and
shared by all webapps, rather than requiring each webapp to bundle the
taglib in its own WEB-INF/lib.
However, scanning all available JARs for TLDs is not very efficient,
especially if the names of the JAR-packaged taglibs are known in
advance.
The proposal is to add a configurable String property (tldJarNames)
on Context, which specifies the comma-separated list of JAR file names
(without any path info) that must be scanned for TLDs.
Catalina will continue to traverse the classloader delegation chain,
but only consider those JARs that match the names in the
comma-separated list.
If a JAR appears more than once in the classloader delegation chain,
we will pick its first occurrence.
If the tldJarNames property is not set, Catalina will continue to scan 
 all JARs in the classloader delegation chain for TLDs.

Comments?

Jan





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [PROPOSAL] Narrow down the list of JARs to be scanned for TLDs

2003-09-22 Thread Jan Luehe
Shapira,

Seems like too much work/complication for a small gain.  How inefficient
is the current mechanism and how much performance would we gain from
your approach?
What do you mean by too much work? :)
I already have a patch ready to be committed. It's just a few line changes.
Just to give you an idea of the proposal's benefit. Currently, we're
scanning the following global JARs for TLDs (depending on your runtime, 
there may be even more!), none of which contain any TLDs:

/home/luehe/ws/jakarta-tomcat-5/build/common/lib/servlet-api.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/commons-dbcp.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/naming-common.jar
/net/koori.sfbay/a/v07/jdk/1.4/fcs/binaries/solsparc/jre/lib/ext/localedata.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/commons-modeler.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/servlets-invoker.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/tomcat-jk.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/naming-resources.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/servlets-webdav.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/jakarta-regexp-1.2.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/commons-beanutils.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/catalina-cluster.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/jkshm.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/endorsed/xmlParserAPIs.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/jasper-compiler.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/tomcat-http11.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/ant.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/naming-factory.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/tomcat-jni.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/jmx-tools.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/catalina-ant.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/catalina-i18n-fr.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/tomcat-jk2.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/commons-logging.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/commons-el.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/jkconfig.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/servlets-default.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/catalina-optional.jar
/net/koori.sfbay/a/v07/jdk/1.4/fcs/binaries/solsparc/jre/lib/ext/sunjce_provider.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/commons-logging-api.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/naming-java.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/catalina.jar
/net/koori.sfbay/a/v07/jdk/1.4/fcs/binaries/solsparc/jre/lib/ext/ldapsec.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/tomcat-util.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/commons-pool.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/commons-digester.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/catalina-i18n-ja.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/catalina-i18n-es.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/tomcat-coyote.jar
/net/koori.sfbay/a/v07/jdk/1.4/fcs/binaries/solsparc/lib/tools.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/commons-fileupload-1.0.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/endorsed/xercesImpl.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/commons-collections.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/jasper-runtime.jar
/net/koori.sfbay/a/v07/jdk/1.4/fcs/binaries/solsparc/jre/lib/ext/dnsns.jar
/home/luehe/ws/jakarta-tomcat-5/build/server/lib/servlets-common.jar
/home/luehe/ws/jakarta-tomcat-5/build/bin/bootstrap.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/jmx.jar
/home/luehe/ws/jakarta-tomcat-5/build/common/lib/jsp-api.jar
Notice that we're scanning this list twice: once for context listeners
(in TldConfig.java), and again in Jasper's TldLocationsCache for
taglibs.
Jan


Yoav Shapira
Millennium ChemInformatics


-Original Message-
From: Jan Luehe [mailto:[EMAIL PROTECTED]
Sent: Monday, September 22, 2003 3:40 PM
To: Tomcat Developers List
Subject: [PROPOSAL] Narrow down the list of JARs to be scanned for TLDs
Currently, any JARs in the classloader delegation chain of a webapp's
classloader are scanned for packaged TLDs. This is convenient, as it
allows a JAR-packaged taglib to be simply dropped in common/lib and
shared by all webapps, rather than requiring each webapp to bundle the
taglib in its own WEB-INF/lib.
However, scanning all available JARs for TLDs is not very efficient,
especially if the names of the JAR-packaged taglibs are known in
advance.
The proposal is to add a configurable String property (tldJarNames)
on Context, which specifies the comma-separated list of JAR file names
(without any path info) that must be scanned for TLDs.
Catalina will continue to traverse the classloader delegation chain,
but only consider those JARs that match the names in the
comma

Re: [PROPOSAL] Narrow down the list of JARs to be scanned for TLDs

2003-09-22 Thread Jan Luehe
Amy Roh wrote:
The proposal is to add a configurable String property (tldJarNames)
on Context, which specifies the comma-separated list of JAR file names
(without any path info) that must be scanned for TLDs.


+1 
Amy :-)
Yeah! Thanks! :)

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core StandardWrapper.java

2003-09-18 Thread Jan Luehe
Remy Maucherat wrote:
[EMAIL PROTECTED] wrote:

luehe   2003/09/17 16:26:33

  Modified:catalina/src/share/org/apache/catalina/core
StandardWrapper.java
  Log:
  Fix for Bugtraq 4924326 (JMX registrations of servlets that map to
  the same jsp-file use the same name)


There was also a similar issue in the request dispatcher, where the 
mapped servlet path was overriden (for no good reason IMO) with the JSP 
file. What was the reasoning behind this code ?
I don't see any reference to StandardWrapper.getJspFile() in
ApplicationDispatcher (other than to set the Globals.JSP_FILE_ATTR
request attribute), so this should not be an issue any more.
Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


cvs commit: jakarta-tomcat-catalina/webapps/docs class-loader-howto.xml

2003-09-08 Thread Jan Luehe
I think the current description of the classloader delegation model
from a web application's perspective is still somewhat misleading.

Currently, the document describes this order:

  /WEB-INF/classes of your web application 
  /WEB-INF/lib/*.jar of your web application 
  Bootstrap classes of your JVM 
  System class loader classses (described above) 
  [...]

Shouldn't this really be:

  Bootstrap classes of your JVM 
  System class loader classses (described above) 
  /WEB-INF/classes of your web application 
  /WEB-INF/lib/*.jar of your web application 
  [...]

Otherwise, Remy's fix of putting commons-logging-api.jar in the system
class loader would not have worked, as org.apache.commons.logging.Log
would be loaded by two different classloaders (the system classloader
and the WebappClassLoader), and

  Log.class.isAssignableFrom(logClass)

in LogFactoryImpl.getLogConstructor would return false.

Also, this would be consistent with the impl of
WebappClassLoader.loadClass, which calls system.loadClass(name)
*before* checking to see if the classloading should be delegated to
the parent, and it would be consistent with a description of how/when
WebappX delegates earlier in the document ([...] There are
exceptions. Classes which are part of the JRE base classes cannot be
overriden [...]).

Also, I think org.apache.commons.logging.*  should no longer be
called out in the list of packages that trigger delegation, as it has
been superseded by placing commons-logging-api.jar in
$CATALINA_HOME/bin.

Following is a patch:


Index: class-loader-howto.xml
===
RCS file: /home/cvs/jakarta-tomcat-catalina/webapps/docs/class-loader-howto.xml,v
retrieving revision 1.5
diff -u -r1.5 class-loader-howto.xml
--- class-loader-howto.xml  8 Sep 2003 13:49:32 -   1.5
+++ class-loader-howto.xml  8 Sep 2003 17:23:56 -
@@ -203,7 +203,6 @@
 liemjavax.*/em/li
 liemorg.xml.sax.*/em/li
 liemorg.w3c.dom.*/em/li
-liemorg.apache.commons.logging.*/em/li
 liemorg.apache.xerces.*/em/li
 liemorg.apache.xalan.*/em/li
 /ul
@@ -214,10 +213,10 @@
 pTherefore, from the perspective of a web application, class or resource
 loading looks in the following repositories, in this order:/p
 ul
-liem/WEB-INF/classes/em of your web application/li
-liem/WEB-INF/lib/*.jar/em of your web application/li
 liBootstrap classes of your JVM/li
 liSystem class loader classses (described above)/li
+liem/WEB-INF/classes/em of your web application/li
+liem/WEB-INF/lib/*.jar/em of your web application/li
 liem$CATALINA_HOME/common/classes/em/li
 liem$CATALINA_HOME/common/endorsed/*.jar/em/li
 liem$CATALINA_HOME/common/lib/*.jar/em/li


Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [5.0] Planning

2003-09-08 Thread Jan Luehe
On that topic, is there a reason that Tomcat 5.0.x still uses
commons-logging-api.jar instead of commons-logging.jar?  If you're 
putting
this jar in common/lib, you'd avoid the need for webapps to have to
include commons-logging.jar themselves in order to use the default
functionality.


Craig, you're funny :-D
The reason is that any setup other than the current one doesn't work ;-)
Facts:
- The c-l API must be in the system CL
- the implementation of a logger must reside in the same CL as the 
logger (ie, if there's log4j somewhere, commons-logging.jar must be next 
to it)
I think the reason why what Craig is suggesting - placing
commons-logging.jar instead of commons-logging-api.jar into
$CATALINA_HOME/bin - would not work is due to an assumption in
commons-logging.jar that its classloader also have access to
log4j.jar.
For instance, the org.apache.commons.logging.impl.Log4JLogger
constructor (in commons-logging.jar) references org.apache.log4j.Logger
(from log4j.jar) as follows:
  import org.apache.log4j.*;

  public Log4JLogger(String name) {
  this.logger=Logger.getLogger(name);
  }
If commons-logging.jar is loaded by the system classloader, it won't
be able to resolve org.apache.log4j.Logger unless log4j.jar is also
loaded by the system classloader, or the above constructor made an
attempt to load org.apache.log4j.Logger
using the context classloader, which is how
org.apache.commons.logging.impl.LogFactoryImpl.isLog4JAvailable()
(in commons-logging-api.jar) loads it:
protected boolean isLog4JAvailable() {

try {
loadClass(org.apache.log4j.Logger);
[...]
}
where loadClass() uses the context classloader: 
getContextClassLoader().loadClass(name).

Therefore, commons-logging.jar and log4j.jar must be placed at the same 
classloading
level, but it is OK for commons-logging-api.jar to be loaded by the 
system classloader (and still be able to detect if log4j is available).

This is what Remy has been saying.

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [PROPOSAL] Add keystoreAlias property to CoyoteConnector

2003-08-14 Thread Jan Luehe
Jan Luehe wrote:
Bill Barker wrote:

Just make certain to close bug #19610 after the commit.


Done.

Notice that 19610 also requests the ability to assign different
passwords to each individual key. JSSE currently does not support
this feature via its standard APIs.
I meant: [...] the ability to assign individual passwords to keys.

Jan


Jan


- Original Message - From: Jan Luehe [EMAIL PROTECTED]
To: Tomcat Developers List [EMAIL PROTECTED]
Sent: Saturday, August 09, 2003 10:38 AM
Subject: [PROPOSAL] Add keystoreAlias property to CoyoteConnector


I would like to add support for specifying a keystore alias property
on CoyoteConnector. This will allow control over which (of possible
many) keypair and supporting cert chain the connector is going to
select to authenticate itself to the client during the SSL handshake,
when client auth is turned on.
If this attribute is specified on the connector, the underlying JSSE
socket factory will initialize the SSL context with a KeyManager
implementation whose methods delegate to the default key manager, with
the exception of the chooseServerAlias method, which will return the
specified alias name.
Let me know if you have any issues with this proposal.

Thanks,

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]




This message is intended only for the use of the person(s) listed 
above as the intended recipient(s), and may contain information that 
is PRIVILEGED and CONFIDENTIAL.  If you are not an intended 
recipient, you may not read, copy, or distribute this message or any 
attachment. If you received this communication in error, please 
notify us immediately by e-mail and then delete all copies of this 
message and any attachments.

In addition you should be aware that ordinary (unencrypted) e-mail 
sent through the Internet is not secure. Do not send confidential or 
sensitive information, such as social security numbers, account 
numbers, personal identification numbers and passwords, to us via 
ordinary (unencrypted) e-mail.





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]




-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit:jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11Http11Protocol.java

2003-08-14 Thread Jan Luehe
Remy Maucherat wrote:
[EMAIL PROTECTED] wrote:

luehe   2003/08/11 14:44:49
  +public void setProtocols(String k) {
  +setAttribute(protocols, k);
  +}
  +


This probably should be sslProtocols, no ?
Hmm, but that would make it inconsistent with Http11Protocol's
setProtocol method, which sets an attribute named protocol.
Remember I raised this issue after adding the sslProtocol property
to CoyoteConnector?
   Added new sslProtocol property + minor javadoc edits

  The protocol property is a little confusing, as it 
means different
  things in different classes:

  - In CoyoteConnector, it refers to the Coyote protocol 
name (i.e.,
HTTP/1.1 or AJP/1.3).

  - In Http11Processor and CoyoteServerSocketFactory, it 
refers to the
SSL protocol variant.

  We may want to fix this.

Well, protocol for HTTP and AJP is the accurate name. 
Protocol for the
secure layer is also the right name (note that this parameter is
remarkably useless, as everything supports TLS now, and the 
old SSL2 is
supposedly insecure).

My understanding is that the property names are mapped as follows:

Connector   Http11Protocol
--
sslProtocol(s)-protocol(s)


Jan





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [PROPOSAL] Add keystoreAlias property to CoyoteConnector

2003-08-14 Thread Jan Luehe
Bill Barker wrote:
Just make certain to close bug #19610 after the commit.
Done.

Notice that 19610 also requests the ability to assign different
passwords to each individual key. JSSE currently does not support
this feature via its standard APIs.
Jan


- Original Message - 
From: Jan Luehe [EMAIL PROTECTED]
To: Tomcat Developers List [EMAIL PROTECTED]
Sent: Saturday, August 09, 2003 10:38 AM
Subject: [PROPOSAL] Add keystoreAlias property to CoyoteConnector



I would like to add support for specifying a keystore alias property
on CoyoteConnector. This will allow control over which (of possible
many) keypair and supporting cert chain the connector is going to
select to authenticate itself to the client during the SSL handshake,
when client auth is turned on.
If this attribute is specified on the connector, the underlying JSSE
socket factory will initialize the SSL context with a KeyManager
implementation whose methods delegate to the default key manager, with
the exception of the chooseServerAlias method, which will return the
specified alias name.
Let me know if you have any issues with this proposal.

Thanks,

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]




This message is intended only for the use of the person(s) listed above as the intended recipient(s), and may contain information that is PRIVILEGED and CONFIDENTIAL.  If you are not an intended recipient, you may not read, copy, or distribute this message or any attachment. If you received this communication in error, please notify us immediately by e-mail and then delete all copies of this message and any attachments.

In addition you should be aware that ordinary (unencrypted) e-mail sent through the Internet is not secure. Do not send confidential or sensitive information, such as social security numbers, account numbers, personal identification numbers and passwords, to us via ordinary (unencrypted) e-mail.





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


[PROPOSAL] Add keystoreAlias property to CoyoteConnector

2003-08-14 Thread Jan Luehe
I would like to add support for specifying a keystore alias property
on CoyoteConnector. This will allow control over which (of possible
many) keypair and supporting cert chain the connector is going to
select to authenticate itself to the client during the SSL handshake,
when client auth is turned on.
If this attribute is specified on the connector, the underlying JSSE
socket factory will initialize the SSL context with a KeyManager
implementation whose methods delegate to the default key manager, with
the exception of the chooseServerAlias method, which will return the
specified alias name.
Let me know if you have any issues with this proposal.

Thanks,

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [VOTE] New committer: Eric Carmichael

2003-08-14 Thread Jan Luehe
+1. I second what Kin-Man said. :)

Jan

Remy Maucherat wrote:
I'd like to nominate Eric Carmichael as a committer on the Tomcat 
project. Eric has been steadily supplying quality patches to the new 
Jasper which will implement the JSP 2.0 specification, and has helped 
fix outstanding bug reports. He plans to continue contributing in the 
future.

He has my +1.

Remy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [Q] Different handling tagfiles and normal tags classes

2003-08-14 Thread Jan Luehe
Torsten,

see JSP.8.3 (Semantics of Tag Files), 3rd bullet:

  For each invocation to the tag, the JSP Context Wrapper must present a
  clean page scope containing no initial elements. All scopes other than
  the page scope must be identical to those in the Invoking JSP Context
  and must be modified accordingly when updates are made to those scopes
  in the JSP Context Wrapper. Any modifications to the page scope,
  however, must not affect the Invoking JSP Context.
Hope this helps.

Jan

Torsten Fohrer wrote:
Tagfiles and tags get a jspcontext from the container at runtime. 
Tags get directly a normal pagecontext reference from the current jsp page. 
TagFiles instead get a wrapper around the pagecontext that wraps all 
set/get/findAttribute for the page scope to a local set of stored
attributes. 
When i declare an attribute in jsp with page scope, i never see it in a tag
file, but i see it in a 'normal' tag.

Interesting files: 
  org.apache.jasper.runtime.JspContextWrapper - see
getAttribute(String)/getAttribute(String/int)
  org.apache.jasper.compiler.Generator - search for setJspContext

Is there a reason why, i don't know? 

Best greeting 
 Torsten Fohrer

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit:jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/net/jsseJSSE14SocketFactory.java

2003-08-09 Thread Jan Luehe
Bill Barker wrote:
 +protected TrustManager[] getTrustManagers(String keystoreType)
 +throws Exception {
 +
 +TrustManager[] tm = null;


Don't you need a check for keystoreType == null here?
Yes, thanks, just added one.

Jan



 +
 +KeyStore trustStore = getTrustStore(keystoreType);
 +if (trustStore != null) {
 +TrustManagerFactory tmf =
TrustManagerFactory.getInstance(SunX509);

 +tmf.init(trustStore);
 +tm = tmf.getTrustManagers();
 +}
 +
 +return tm;
 +}
  }


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]




This message is intended only for the use of the person(s) listed above as the intended recipient(s), and may contain information that is PRIVILEGED and CONFIDENTIAL.  If you are not an intended recipient, you may not read, copy, or distribute this message or any attachment. If you received this communication in error, please notify us immediately by e-mail and then delete all copies of this message and any attachments.

In addition you should be aware that ordinary (unencrypted) e-mail sent through the Internet is not secure. Do not send confidential or sensitive information, such as social security numbers, account numbers, personal identification numbers and passwords, to us via ordinary (unencrypted) e-mail.





-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


[PROPOSAL] Expose pre-compiled JSPs without servlet mapping

2003-07-23 Thread Jan Luehe
Currently, if webapp developers do not want to expose the source of
their JSP files, they have to precompile them and add a servlet
mapping for each JSP to their web.xml (e.g., with the help of jspc).
If the webapp contains a large number of JSPs, the web.xml is going to
grow pretty big.
Would it make sense to have Jasper try to load a class file
corresponding to a JSP, even if the webapp does not have the JSP
source file and does not specify any servlet mapping for that JSP?
So if someone accesses a JSP for which there is no servlet mapping,
the JspServlet will first determine if the JSP exists, and if it
doesn't, it will try to load the corresponding class file. If the class
file does not exist, a 404 is returned.
This will eliminate the need for adding servlet mappings for
precompiled JSPs to the web.xml.
One advantage of having the servlet mappings is that precompiled JSPs
may be selectively exposed, whereas with the proposed scheme, *any*
precompiled JSP would be exposed. We could define a config option in
JspServlet that would disable the proposed behaviour and require a
servlet mapping in order for a precompiled JSP to be exposed.
Comments?

Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit:jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/servletJspServlet.java

2003-07-23 Thread Jan Luehe
Remy,

So I guess we have no choice but make it configurable (or not support
it at all, which I don't think is an option), but since you've agreed
that we should make this a configurable option (provided it is added
at the right places), this is no longer an issue.
I just committed a revised impl of this (and reverted my previous 
commit).


Ok, cool :)
one comment, though: By adding the xpoweredBy property to 
CoyoteConnector instead of Host/Context, we get the

  X-Powered-By: Servlet/2.4

response header even for static resources. You could argue that this is 
OK, since static resources are handled by the DefaultServlet, but it may 
not be very intuitive.

By defining the xpoweredBy property on Host/Context instead, and adding 
the response header in StandardWrapperValve (as in the original commit), 
we could suppress the header if the servlet being invoked is an instance 
of org.apache.catalina.servlets.DefaultServlet.

What do you think?

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: [PROPOSAL] Expose pre-compiled JSPs without servlet mapping

2003-07-23 Thread Jan Luehe
Tim Funk wrote:
How will phantom pages be addressed? Pages where the jsp once existed 
but then was deleted, but the corresponding class was not deleted?
That's why I suggested an option for JspServlet that would disable this 
optimization and require a servlet mapping for each precompiled JSP.

But your're right, in the case where the optimization is enabled, 
phantom pages will be served, so you better know what you're doing when 
turning on this optimization.

However, based on your and Remy's comments, I've decided to not add this 
feature: too much headache for a small gain (reduced web.xml size).

Thanks,

Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit:jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/servletJspServlet.java

2003-07-23 Thread Jan Luehe

Arg, what a hack ;-) (and my definition is that we get into the servlet 
container; if it's an internal servlet, it seems good enough, and the 
page being served was served by a Servlet API 2.4 component)
Come on, this is a stupid feature nobody but marketting cares about (and 
I'd say the said marketting folks should forget about it: Netcraft can 
count Java powered websites well enough already). I suppose the 
reasoning is that IIS/6.0 adds similar self agrandizing headers that it 
is .NET 1.1 powered, so we have to do the same. Lame.
So the implementation of the feature has IMO to be as invisible as 
possible given its lack of usefulness.
OK, I was just trying to make sure we didn't miss anything. :)
Like I said (and which you confirmed), the DefaultServlet is a servlet, 
so adding the response header (even) to static resources may not be too 
far-fetched.

Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit:jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/servletJspServlet.java

2003-07-22 Thread Jan Luehe
Remy Maucherat wrote:

 BTW, I don't see why the spec saying that the header is optional implies
 that the flag must be implemented as something optional. It merely means
 that an implementation may ignore completely this feature.
Actually, the Servlet spec is going to say this:

  Note that this header [X-Powered-By] is optional, but if the
  container supports this feature, it must be configurable to suppress this
  header.
So I guess we have no choice but make it configurable (or not support
it at all, which I don't think is an option), but since you've agreed
that we should make this a configurable option (provided it is added
at the right places), this is no longer an issue.
I just committed a revised impl of this (and reverted my previous commit).

Jan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit:jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/servletJspServlet.java

2003-07-21 Thread Jan Luehe
  +
  +/*
  + * Add X-Powered-By header for JSP, if Catalina already 
added a
  + * corresponding header for servlets
  + */
  +if (response.containsHeader(X-Powered-By)) {
  +response.addHeader(X-Powered-By, JSP/2.0);
  +}
  +


This is a pretty bad implementation IMO.
What's the use of disabling this feature ?
The spec declares these headers as optional, which means Tomcat should 
make them configurable. Some sites may prefer not to include this 
information in their responses, for security reasons or whatever.

-1 on the various flags and checks (just add the headers, without flags 
and complexity). -0 if you can indicate a good reason for this.
Also, you shouldn't add the JSP 2.0 header in JspServlet. If you 
precompile, it's not called. Put it in HttpJspBase.
I had thought about adding the JSP 2.0 header in HttpJspBase, but then 
realized that the extends page directive allows you to specify the 
class that the generated servlet should extend, in which case 
HttpJspBase will be out of the picture. Do you have a better idea?

Jan



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


  1   2   >