Hi Mark,

Mark Thomas wrote On 01/25/07 04:33 PM,:

 Jan Luehe wrote:

> please go to https://servlet-spec-eg.dev.java.net/ and request
> observer role.


 I tried by I can't access any of the project. My id is medthomas.
 Could you forward the request for me?

I'm forwarding the message I just sent to the EG, which has
the proposed diffs.

Thanks,

Jan



 Many thanks,

 Mark

--- Begin Message ---
Based on feedback from Mike Kaufman (see
https://servlet-spec-eg.dev.java.net/issues/show_bug.cgi?id=40),
and a bug I discovered, I propose that we make the following
backwards-compatible changes to javax.servlet.http.HttpServlet.

I've attached the diffs, and also copied the relevant parts of the
code that are going to change, where the diffs are hard to read.

Summary of changes:

- Have NoBodyResponse extend HttpServletResponseWrapper, which will
 avoid a lot of redundant implementation code for this class.
 With this change, NoBodyResponse will continue to implement
 HttpServletResponse.

 Make sure that calling getOutputStream() after getWriter() (or vice
 versa) results in an IllegalStateException, as required by the servlet
 spec.

 Also make sure that if getWriter() has been called, the writer is
 flushed before the getContentLength() method of the writer's underlying
 NoBodyOutputStream is called (otherwise, the resulting Content-Length
 will always be equal to zero).

 Here are the relevant code changes for NoBodyResponse:

 CHANGE:

   class NoBodyResponse implements HttpServletResponse {

 TO:

   class NoBodyResponse extends HttpServletResponseWrapper {

 ----------------------------------------------------------------
 CHANGE:

   NoBodyResponse(HttpServletResponse r) {
       resp = r;
       noBody = new NoBodyOutputStream();
   }

 TO

   NoBodyResponse(HttpServletResponse r) {
       super(r);
       noBody = new NoBodyOutputStream();
   }

 ----------------------------------------------------------------
 CHANGE:

   void setContentLength() {
       if (!didSetContentLength)
         resp.setContentLength(noBody.getContentLength());
   }

 TO:

   void setContentLength() {
       if (!didSetContentLength) {
           if (writer != null) {
               writer.flush();
           }
           setContentLength(noBody.getContentLength());
       }
   }

 ----------------------------------------------------------------
 CHANGE:

   public ServletOutputStream getOutputStream() throws IOException
     { return noBody; }

 TO:

   public ServletOutputStream getOutputStream() throws IOException {

       if (writer != null) {
           throw new IllegalArgumentException(
               lStrings.getString("err.ise.getOutputStream"));
       }
       usingOutputStream = true;

       return noBody;
   }

 ----------------------------------------------------------------
 CHANGE:

   public PrintWriter getWriter() throws UnsupportedEncodingException
   {
       if (writer == null) {
           OutputStreamWriter  w;

           w = new OutputStreamWriter(noBody, getCharacterEncoding());
           writer = new PrintWriter(w);
       }
       return writer;
   }

 TO:

   public PrintWriter getWriter() throws UnsupportedEncodingException {

       if (usingOutputStream) {
           throw new IllegalArgumentException(
               lStrings.getString("err.ise.getWriter"));
       }

       if (writer == null) {
           OutputStreamWriter w = new OutputStreamWriter(
               noBody, getCharacterEncoding());
           writer = new PrintWriter(w);
       }

       return writer;
   }

- Added comment explaining why NoBodyOutputStream.write() really should
 have thrown an IllegalArgumentException (instead of IOException) on
 negative lengths to begin with, and why changing this would break
 backwards compatibility.

- Have NoBodyOutputStream.write() throw the localized error message
 corresponding to the "err.io.negativelength" error code.

- Removed the "close();" and "return;" statements from HttpServlet.doTrace()


Please let me know if you have any concerns about these changes.

Thanks,


Jan

ld.so.1: /usr/bin/sparcv9/uptime: warning: libsocks5_sh.so: open failed: No 
such file in secure directories
Index: HttpServlet.java
===================================================================
RCS file: 
/cvs/glassfish/servlet-api/src/jakarta-servletapi-5/jsr154/src/share/javax/servlet/http/HttpServlet.java,v
retrieving revision 1.3
diff -u -r1.3 HttpServlet.java
--- HttpServlet.java    14 Mar 2006 21:28:49 -0000      1.3
+++ HttpServlet.java    26 Jan 2007 00:49:46 -0000
@@ -279,7 +279,7 @@
        NoBodyResponse response = new NoBodyResponse(resp);
        
        doGet(req, response);
-       response.setContentLength();
+        response.setContentLength();
     }
     
 
@@ -654,8 +654,6 @@
        resp.setContentLength(responseLength);
        ServletOutputStream out = resp.getOutputStream();
        out.print(responseString);      
-       out.close();
-       return;
     }          
 
 
@@ -828,172 +826,69 @@
  * A response that includes no body, for use in (dumb) "HEAD" support.
  * This just swallows that body, counting the bytes in order to set
  * the content length appropriately.  All other methods delegate directly
- * to the HTTP Servlet Response object used to construct this one.
+ * to the wrapped HTTP Servlet Response object.
  */
 // file private
-class NoBodyResponse implements HttpServletResponse {
-    private HttpServletResponse                resp;
+class NoBodyResponse extends HttpServletResponseWrapper {
+
+    private static final ResourceBundle lStrings
+        = ResourceBundle.getBundle("javax.servlet.http.LocalStrings");
+
     private NoBodyOutputStream         noBody;
     private PrintWriter                        writer;
     private boolean                    didSetContentLength;
+    private boolean                     usingOutputStream;
 
     // file private
     NoBodyResponse(HttpServletResponse r) {
-       resp = r;
+       super(r);
        noBody = new NoBodyOutputStream();
     }
 
     // file private
     void setContentLength() {
-       if (!didSetContentLength)
-         resp.setContentLength(noBody.getContentLength());
+        if (!didSetContentLength) {
+            if (writer != null) {
+                writer.flush();
+            }
+            setContentLength(noBody.getContentLength());
+        }
     }
 
-
-    // SERVLET RESPONSE interface methods
-
     public void setContentLength(int len) {
-       resp.setContentLength(len);
-       didSetContentLength = true;
+        super.setContentLength(len);
+        didSetContentLength = true;
     }
 
-    public void setCharacterEncoding(String charset)
-      { resp.setCharacterEncoding(charset); }
-
-    public void setContentType(String type)
-      { resp.setContentType(type); }
-
-    public String getContentType()
-      { return resp.getContentType(); }
+    public ServletOutputStream getOutputStream() throws IOException {
 
-    public ServletOutputStream getOutputStream() throws IOException
-      { return noBody; }
-
-    public String getCharacterEncoding()
-       { return resp.getCharacterEncoding(); }
-
-    public PrintWriter getWriter() throws UnsupportedEncodingException
-    {
-       if (writer == null) {
-           OutputStreamWriter  w;
+        if (writer != null) {
+            throw new IllegalArgumentException(
+                lStrings.getString("err.ise.getOutputStream"));
+        }
+        usingOutputStream = true;
 
-           w = new OutputStreamWriter(noBody, getCharacterEncoding());
-           writer = new PrintWriter(w);
-       }
-       return writer;
+        return noBody;
     }
 
-    public void setBufferSize(int size) throws IllegalStateException
-      { resp.setBufferSize(size); }
-
-    public int getBufferSize()
-      { return resp.getBufferSize(); }
-
-    public void reset() throws IllegalStateException
-      { resp.reset(); }
-      
-      public void resetBuffer() throws IllegalStateException
-      { resp.resetBuffer(); }
-
-    public boolean isCommitted()
-      { return resp.isCommitted(); }
-
-    public void flushBuffer() throws IOException
-      { resp.flushBuffer(); }
-
-    public void setLocale(Locale loc)
-      { resp.setLocale(loc); }
-
-    public Locale getLocale()
-      { return resp.getLocale(); }
-
-
-    // HTTP SERVLET RESPONSE interface methods
-
-    public void addCookie(Cookie cookie)
-      { resp.addCookie(cookie); }
-
-    public boolean containsHeader(String name)
-      { return resp.containsHeader(name); }
-
-    /** @deprecated */
-    public void setStatus(int sc, String sm)
-      { resp.setStatus(sc, sm); }
-
-    public void setStatus(int sc)
-      { resp.setStatus(sc); }
-
-    public void setHeader(String name, String value)
-      { resp.setHeader(name, value); }
-
-    public void setIntHeader(String name, int value)
-      { resp.setIntHeader(name, value); }
+    public PrintWriter getWriter() throws UnsupportedEncodingException {
 
-    public void setDateHeader(String name, long date)
-      { resp.setDateHeader(name, date); }
-
-    public void sendError(int sc, String msg) throws IOException
-      { resp.sendError(sc, msg); }
-
-    public void sendError(int sc) throws IOException
-      { resp.sendError(sc); }
-
-    public void sendRedirect(String location) throws IOException
-      { resp.sendRedirect(location); }
-    
-    public String encodeURL(String url) 
-      { return resp.encodeURL(url); }
-
-    public String encodeRedirectURL(String url)
-      { return resp.encodeRedirectURL(url); }
-      
-    public void addHeader(String name, String value)
-      { resp.addHeader(name, value); }
-      
-    public void addDateHeader(String name, long value)
-      { resp.addDateHeader(name, value); }
-      
-    public void addIntHeader(String name, int value)
-      { resp.addIntHeader(name, value); }
-      
-      
-      
-
-    /**
-     * @deprecated     As of Version 2.1, replaced by
-     *                         [EMAIL PROTECTED] 
HttpServletResponse#encodeURL}.
-     *
-     */
-     
-     
-    public String encodeUrl(String url) 
-      { return this.encodeURL(url); }
-      
-      
-      
-      
-      
-      
-      
+        if (usingOutputStream) {
+            throw new IllegalArgumentException(
+                lStrings.getString("err.ise.getWriter"));
+        }
 
-    /**
-     * @deprecated     As of Version 2.1, replaced by
-     *                 [EMAIL PROTECTED] 
HttpServletResponse#encodeRedirectURL}.
-     *
-     */
-     
-     
-    public String encodeRedirectUrl(String url)
-      { return this.encodeRedirectURL(url); }
+        if (writer == null) {
+            OutputStreamWriter w = new OutputStreamWriter(
+                noBody, getCharacterEncoding());
+            writer = new PrintWriter(w);
+        }
 
+        return writer;
+    }
 }
 
 
-
-
-
-
-
 /*
  * Servlet output stream that gobbles up all its data.
  */
@@ -1026,11 +921,9 @@
        if (len >= 0) {
            contentLength += len;
        } else {
-           // XXX
-           // isn't this really an IllegalArgumentException?
-           
-           String msg = lStrings.getString("err.io.negativelength");
-           throw new IOException("negative length");
+            // This should have thrown an IllegalArgumentException, but
+            // changing this would break backwards compatibility
+            throw new IOException(lStrings.getString("err.io.negativelength"));
        }
     }
 }
ld.so.1: /usr/bin/sparcv9/uptime: warning: libsocks5_sh.so: open failed: No 
such file in secure directories
Index: LocalStrings.properties
===================================================================
RCS file: 
/cvs/glassfish/servlet-api/src/jakarta-servletapi-5/jsr154/src/share/javax/servlet/http/LocalStrings.properties,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 LocalStrings.properties
--- LocalStrings.properties     27 May 2005 22:57:32 -0000      1.1.1.1
+++ LocalStrings.properties     26 Jan 2007 00:51:22 -0000
@@ -2,8 +2,10 @@
 # Localized for Locale en_US
 
 err.cookie_name_is_token=Cookie name \"{0}\" is a reserved token
-err.io.negativelength=Negative Length given in write method
+err.io.negativelength=Negative length given in write method
 err.io.short_read=Short Read
+err.ise.getWriter=Illegal to call getWriter() after getOutputStream() has been 
called
+err.ise.getOutputStream=Illegal to call getOutputStream() after getWriter() 
has been called
 
 http.method_not_implemented=Method {0} is not defined in RFC 2068 and is not 
supported by the Servlet API 
 

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

Reply via email to