tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Cruikshank <a...@epitonic.com>
Subject Bugs in JspReader.java
Date Wed, 01 Dec 1999 19:21:16 GMT
Sorry for such a long email.  I read the jakarta contributions page, but I 
am still unclear how to bugs and suggested fixes as a non-committer.  If 
this is inappropriate, please let me know.  Otherwise, I would greatly 
appreciate someone looking into these changes.

In the process of trying to port a GnuJSP application over to Jakarta, I 
encountered several problems revolving around included directives at the 
end of the JSP page.  Specifically the page include.jsp:

<@ page...>
...
<%@ include file="foo.jsp" %>

throws a ArrayIndexOutOfBoundsException if there is no trailing space after 
the include.  It appears the cause is a bug in the hasMoreInput() method of 
JspReader.  The existing code:

     public boolean hasMoreInput() {
         if (current.cursor >= stream.length)
         {
             return popFile();
         }
         return true;
     }

Does not handle the above case correctly, because, upon reaching the end of 
foo.jsp, it returns the value of popFile() which is true because 
include.jsp was still on the stack.  The cursor of include.jsp, however, is 
already past the end of it's stream, so it does not really have more 
input.  In practice, this violation of hasMoreInput()'s contract causes the 
Parser to attempt to match another tag in include.jsp and throw an array 
exception when the Tag parser tries a peekChar() on the now empty 
JspReader.  I think it's questionable that peekChar() should access the 
stream array without checking if it is in bounds (although it should be ok 
as long as hasMoreInput() is called first and works as one would 
expect).  This problem can be fixed by modifying hasMoreInput as follows:


     public boolean hasMoreInput()
     {
         if (current.cursor >= stream.length)
         {
             while (popFile())
             {
                 if ( current.cursor < stream.length) return true;
             }
             return false;
         }
	 return true;
     }


Unfortunately this is not the only problem.  If the file include.jsp is 
included in yet another file test.jsp:

<%@ page .... >
...
<%@ include file="include.jsp" %>
...

Some part of test.jsp might be missing from the output of the Jsp compiler 
(or an exception might be thrown).  This is because match() and 
matchIgnoreCase() in JspReader looks like this:

     public boolean matchesIgnoreCase(String string) {
         Mark mark = mark();
         int ch = nextChar();
         int i = 0;
         do {
	    if (... != string.charAt(i++)) {
		reset(mark);
		return false;
	    }
            ch = nextChar();
         } while (i < string.length());
         reset(mark);
	 return true;
     }

If the match is successful, the method will get an extra character after 
the end of the match before reseting the mark.  In the case where match is 
asked to match the "%>" at the end of the include in include.jsp, the extra 
character does not exist, so getChar() calls hasMoreInput() which calls 
popFile() which sets the stream to test.jsp's stream and sets the cursor 
(current) to the character after the include in test.jsp.  When match() 
then calls reset, it only resets the cursor (not the stream).  So instead 
of returning with the state set to the begining of the "%>" in include.jsp, 
match() returns with the state set to an arbitrary character in test.jsp.
     The quick fix is to change match() and matchIgnoreCase() so they don't 
get the extra character (which is more efficient anyway):

     public boolean matchesIgnoreCase(String string)
     {
         Mark mark = mark();
         int ch = 0;
         int i = 0;
         do {
            ch = nextChar();
	    if (... != string.charAt(i++)) {
		reset(mark);
		return false;
	    }
         } while (i < string.length());
         reset(mark);
	 return true;
     }

This will probably fix the problem in all valid Jsp pages, but it will 
still cause unpredictable results under error conditions (eg. it finds 
"<jsp:pa" at the end of an included file).  The better solution would be to 
create an internal class TotalState:

     class TotalState
     {
	Stack includeStack;
	Mark current;
	char[] stream = null;

	void restore()
        {
	    JspReader.this.current = current;
	    JspReader.this.stream = stream;
            JspReader.this.includeStack = includeStack;
	}
	
	TotalState()
        {
	    this.current = JspReader.this.current;
	    this.stream = JspReader.this.stream;

            // create include stack
            if ( JspReader.this.includeStack != null )
            {
                includeStack = new Stack();

                // clone the existing includeStack without cloning it's 
contents
                for (int i=0; i < JspReader.this.includeStack.size(); i++)
                {
                    includeStack.addElement( 
JspReader.this.includeStack.elementAt(i) );
                }
            }
	 }
     }


Then, every where a look-ahead is required use:

TotalState state = new TotalState();
...
state.restore();

intead of:

Mark mark = mark();
...
reset( mark );



Hope this helps.
Alex Cruikshank
software engineer
Epitonic.com
(alex@eptionic.com)

Mime
View raw message