Bug 43757 - Improper parsing of response.sendRedirect() in JSP source
Summary: Improper parsing of response.sendRedirect() in JSP source
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 5
Classification: Unclassified
Component: Jasper (show other bugs)
Version: Nightly Build
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-10-31 11:55 UTC by Charles Stahl
Modified: 2008-01-06 15:14 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Charles Stahl 2007-10-31 11:55:53 UTC
Tomcat is throwing a java.lang.illegalStateException due to an attempted
response.sendRedirect() after the time for such matters has come and gone...
But, this appears to be a symptom of bad .jsp parsing at runtime.

==========================

org.apache.jasper.JasperException: Exception in JSP: /buslog.jsp:42

39: // if (session.getAttribute("isAuth") == null) {
40: //     session.setAttribute("messageToUser","Authentication failed.");
41: //        you.shouldNotParseThis();
42: //     response.sendRedirect("./index.jsp");
43: // }
44: 
45: // if (session.getAttribute("isAuth") != null) {

==============================

These lines are commented out in the source, and should not conceivably cause a
runtime error. Changing "response.sendRedirect(...)" to
"response.sendAbendOMG(...)" suppresses the error. I am willing to provide the
complete source if necessary.

Build and execution environments are NetBeans 5.5.1 with bundled Tomcat 5.5.17.
Comment 1 Mark Thomas 2007-10-31 16:48:05 UTC
I am tempted to think that the source Tomcat is parsing isn't the source you
think it is. I can't reproduce the issue. Can you provide a simple JSP that
exhibits this problem?
Comment 2 Charles Stahl 2007-10-31 17:26:06 UTC
(In reply to comment #1)
> I am tempted to think that the source Tomcat is parsing isn't the source you
> think it is. I can't reproduce the issue. Can you provide a simple JSP that
> exhibits this problem?

Greetings Mark :)

Here is a .jsp that should allow you to reproduce the issue:

=========================

<%@page contentType="text/html"%>
<%@page pageEncoding="UTF-8"%>
<%@page import="java.util.*"%>
<%@page import="javax.servlet.*"%>
<%


ServletContext context = this.getServletContext();



// if (session.getAttribute("isAuth") == null) {
//     session.setAttribute("messageToUser","Authentication failed.");
//        you.shouldNotParseThis();
//     response.sendRedirect("./index.jsp");
// }

// if (session.getAttribute("isAuth") != null) {
//     if (!session.getAttribute("isAuth").toString().equals("true")) {
//        session.setAttribute("messageToUser","Authentication failed.");
//         response.sendRedirect("./index.jsp");
//     }
// }

String state = "";

if (request.getParameter("state") != null) {
    state = request.getParameter("state");
} else {
    response.sendRedirect("./index.jsp");
}

// each state is caught by name, final fall through goes back to index.jsp
if (state.equals("CRInput")) {
    response.sendRedirect("./crinput.jsp");
} else {
    response.sendRedirect("./index.jsp");
}


%>

==============================

There is actually a logical flaw with this (possible for two
response.sendRedirect calls), and that is the root cause of the exception... my
bad! But, the HTTP 500 response incorrectly points to the line number of the
first occurrence of response.sendRedirect, whether it is commented out or not.
This makes debugging quite a pain.

With my recent discovery of the cause of the exception I feel that I set the
severity of this a bit high. I was not sure if it was my place to correct it, so
demote away if you'd like.

Thank you for the swift response!
Comment 3 Tim Funk 2007-10-31 17:32:55 UTC
Look at the generated java a little closer.

You'll notice after the commented code something like this: 
out.println("\n")

Because there is whitespace at the end of the file after the %>

So if one tried to write AFTER a redirect is done - you get an ISE.

You need
if (someCondition) {
  response.sendRedirect("./index.jsp");
  return;
}
Comment 4 Charles Stahl 2007-10-31 19:02:03 UTC
(In reply to comment #3)
> Look at the generated java a little closer.
> You'll notice after the commented code something like this: 
> out.println("\n")
> Because there is whitespace at the end of the file after the %>
> So if one tried to write AFTER a redirect is done - you get an ISE.
> You need
> if (someCondition) {
>   response.sendRedirect("./index.jsp");
>   return;
> }

You are correct, and I noted the cause (a little less verbosely) in my reply 
to Mark Thomas' response. However, the error generated does not properly 
indicate the line number that generated the error. That is what I'm driving at.
Comment 5 Mark Thomas 2007-10-31 20:52:45 UTC
Yep - looks like we aren't counting commented out lines correctly during
generation. Thanks for the test case - I'll take a look.

Updating properties accordingly.
Comment 6 Mark Thomas 2007-11-29 14:34:11 UTC
On inspection this wasn't an SMAP problem with comments but an issue with how we
were finding the right JSP line given the Java line.

I have fixed it in trunk and proposed it for backport to 6.0.x and 5.5.x
Comment 7 Mark Thomas 2008-01-06 15:14:12 UTC
Fixed in 5.5.x and will be included in 5.5.26 onwards.