Updated Branches: refs/heads/wicket-1.5.x 7308d624b -> 7b711dcda
WICKET-4927 merged from trunk Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/7b711dcd Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/7b711dcd Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/7b711dcd Branch: refs/heads/wicket-1.5.x Commit: 7b711dcdadd4386039feb4f7ea2ec93ed409f9a0 Parents: 7308d62 Author: svenmeier <[email protected]> Authored: Fri Dec 14 16:31:49 2012 +0100 Committer: svenmeier <[email protected]> Committed: Fri Dec 14 16:31:49 2012 +0100 ---------------------------------------------------------------------- .../protocol/http/HeaderBufferingWebResponse.java | 98 ++++++----- .../http/HeaderBufferingWebResponseTest.java | 119 ++++--------- .../protocol/http/ResponseIOExceptionTest.java | 134 +++++++++++++++ .../wicket/request/HttpHeaderCollection.java | 56 +++--- 4 files changed, 247 insertions(+), 160 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/7b711dcd/wicket-core/src/main/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponse.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponse.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponse.java index 5463114..44c5073 100644 --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponse.java +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponse.java @@ -24,135 +24,134 @@ import org.apache.wicket.util.lang.Args; import org.apache.wicket.util.time.Time; /** - * Response that keeps headers in buffers but writes the content directly to the response. + * Response that keeps headers in buffers until the first content is written. * * This is necessary to get {@link #reset()} working without removing the JSESSIONID cookie. When * {@link HttpServletResponse#reset()} is called it removes all cookies, including the JSESSIONID - * cookie. + * cookie - see <a href="https://issues.apache.org/bugzilla/show_bug.cgi?id=26183";>Bug 26183</a>. * - * Calling {@link #reset()} on this response only clears the buffered headers. If there is any - * content written to response it throws {@link IllegalStateException}. + * Calling {@link #reset()} on this response clears the buffered meta data, if there is already any + * content written it throws {@link IllegalStateException}. * * @author Matej Knopp */ class HeaderBufferingWebResponse extends WebResponse implements IMetaDataBufferingWebResponse { private final WebResponse originalResponse; + + /** + * Buffer of meta data. + */ private final BufferedWebResponse bufferedResponse; public HeaderBufferingWebResponse(WebResponse originalResponse) { this.originalResponse = originalResponse; + bufferedResponse = new BufferedWebResponse(originalResponse); } - private boolean bufferedWritten = false; + private boolean buffering = true; - private void writeBuffered() + private void stopBuffering() { - if (!bufferedWritten) + if (buffering) { bufferedResponse.writeTo(originalResponse); - bufferedWritten = true; + buffering = false; } } - private void checkHeader() + /** + * The response used for meta data. + * + * @return buffered response if nothing was written yet, the original response otherwise + */ + private WebResponse getMetaResponse() { - if (bufferedWritten) + if (buffering) + { + return bufferedResponse; + } + else { - throw new IllegalStateException("Header was already written to response!"); + return originalResponse; } } @Override public void addCookie(Cookie cookie) { - checkHeader(); - bufferedResponse.addCookie(cookie); + getMetaResponse().addCookie(cookie); } @Override public void clearCookie(Cookie cookie) { - checkHeader(); - bufferedResponse.clearCookie(cookie); + getMetaResponse().clearCookie(cookie); } - private boolean flushed = false; - @Override public void flush() { - if (!bufferedWritten) - { - bufferedResponse.writeTo(originalResponse); - bufferedResponse.reset(); - } + stopBuffering(); + originalResponse.flush(); - flushed = true; } @Override public boolean isRedirect() { - return bufferedResponse.isRedirect(); + return getMetaResponse().isRedirect(); } @Override public void sendError(int sc, String msg) { - checkHeader(); - bufferedResponse.sendError(sc, msg); + getMetaResponse().sendError(sc, msg); } @Override public void sendRedirect(String url) { - checkHeader(); - bufferedResponse.sendRedirect(url); + getMetaResponse().sendRedirect(url); } @Override public void setContentLength(long length) { - checkHeader(); - bufferedResponse.setContentLength(length); + getMetaResponse().setContentLength(length); } @Override public void setContentType(String mimeType) { - checkHeader(); - bufferedResponse.setContentType(mimeType); + getMetaResponse().setContentType(mimeType); } @Override public void setDateHeader(String name, Time date) { Args.notNull(date, "date"); - checkHeader(); - bufferedResponse.setDateHeader(name, date); + getMetaResponse().setDateHeader(name, date); } @Override public void setHeader(String name, String value) { - checkHeader(); - bufferedResponse.setHeader(name, value); + getMetaResponse().setHeader(name, value); } @Override public void addHeader(String name, String value) { - checkHeader(); - bufferedResponse.addHeader(name, value); + getMetaResponse().addHeader(name, value); } @Override public void setStatus(int sc) { - bufferedResponse.setStatus(sc); + getMetaResponse().setStatus(sc); } @Override @@ -170,14 +169,16 @@ class HeaderBufferingWebResponse extends WebResponse implements IMetaDataBufferi @Override public void write(CharSequence sequence) { - writeBuffered(); + stopBuffering(); + originalResponse.write(sequence); } @Override public void write(byte[] array) { - writeBuffered(); + stopBuffering(); + originalResponse.write(array); } @@ -185,19 +186,24 @@ class HeaderBufferingWebResponse extends WebResponse implements IMetaDataBufferi @Override public void write(byte[] array, int offset, int length) { - writeBuffered(); + stopBuffering(); + originalResponse.write(array, offset, length); } @Override public void reset() { - if (flushed) + if (buffering) + { + // still buffering so just reset the buffer of meta data + bufferedResponse.reset(); + } + else { - throw new IllegalStateException("Response has already been flushed!"); + // the original response is never reset (see class javadoc) + throw new IllegalStateException("Response is no longer buffering!"); } - bufferedResponse.reset(); - bufferedWritten = false; } public void writeMetaData(WebResponse webResponse) http://git-wip-us.apache.org/repos/asf/wicket/blob/7b711dcd/wicket-core/src/test/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponseTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponseTest.java b/wicket-core/src/test/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponseTest.java index 6c0bca7..1383b4b 100644 --- a/wicket-core/src/test/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponseTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponseTest.java @@ -16,120 +16,67 @@ */ package org.apache.wicket.protocol.http; -import java.net.SocketException; - -import javax.servlet.http.HttpServletResponse; - -import org.apache.wicket.protocol.http.servlet.ResponseIOException; -import org.apache.wicket.protocol.http.servlet.ServletWebRequest; -import org.apache.wicket.protocol.http.servlet.ServletWebResponse; -import org.apache.wicket.request.IRequestHandler; -import org.apache.wicket.request.Response; -import org.apache.wicket.request.cycle.AbstractRequestCycleListener; -import org.apache.wicket.request.cycle.RequestCycle; -import org.apache.wicket.request.handler.EmptyRequestHandler; -import org.apache.wicket.request.resource.ResourceStreamResource; -import org.apache.wicket.util.resource.StringResourceStream; -import org.apache.wicket.util.tester.WicketTester; -import org.junit.After; +import org.apache.wicket.mock.MockWebResponse; import org.junit.Assert; -import org.junit.Before; import org.junit.Test; /** - * @author Pedro Santos + * Test for {@link HeaderBufferingWebResponse}. + * + * @author svenmeier */ public class HeaderBufferingWebResponseTest extends Assert { - private WicketTester tester; /** - * @throws Exception + * WICKET-4927 */ - @Before - public void before() throws Exception + @Test + public void additionalHeaderAfterWrittenContent() { - tester = new WicketTester() - { - @Override - protected Response newServletWebResponse(ServletWebRequest servletWebRequest) - { - return new HeaderBufferingWebResponse(new ProblematicResponse(servletWebRequest, - getResponse())); - } - }; - tester.setExposeExceptions(false); - } + MockWebResponse originalResponse = new MockWebResponse(); - /** - * @throws Exception - */ - @After - public void after() throws Exception - { - tester.destroy(); + HeaderBufferingWebResponse response = new HeaderBufferingWebResponse(originalResponse); + + response.addHeader("key1", "value1"); + + assertNull(originalResponse.getHeader("key1")); + + response.write("written"); + + assertEquals("value1", originalResponse.getHeader("key1")); + + response.addHeader("key2", "value2"); + + assertEquals("value2", originalResponse.getHeader("key2")); } /** - * WICKET-3570 */ @Test - public void giveUpRespondingOnIOExceptions() + public void resetAfterWrittenContent() { - TestRequestCycleListener testRequestCycleListener = new TestRequestCycleListener(); - tester.getApplication().getRequestCycleListeners().add(testRequestCycleListener); - tester.startResource(new ResourceStreamResource(new StringResourceStream("asdf"))); - assertTrue(testRequestCycleListener.lastExceptionRquestHandlerResolved instanceof EmptyRequestHandler); - } + MockWebResponse originalResponse = new MockWebResponse(); - class TestRequestCycleListener extends AbstractRequestCycleListener - { - IRequestHandler lastExceptionRquestHandlerResolved; + HeaderBufferingWebResponse response = new HeaderBufferingWebResponse(originalResponse); - @Override - public void onExceptionRequestHandlerResolved(RequestCycle cycle, IRequestHandler handler, - Exception exception) - { - lastExceptionRquestHandlerResolved = handler; - } + response.addHeader("key1", "value1"); - } - /** - * Mock response simulating connection lost problems. - */ - public static class ProblematicResponse extends ServletWebResponse - { + assertNull(originalResponse.getHeader("key1")); - /** - * @param webRequest - * @param httpServletResponse - */ - public ProblematicResponse(ServletWebRequest webRequest, - HttpServletResponse httpServletResponse) - { - super(webRequest, httpServletResponse); - } + response.reset(); - @Override - public void flush() - { - throw new ResponseIOException(new SocketException( - "Connection reset by peer: socket write error")); - } + response.write("written"); - @Override - public void write(byte[] array) + try { - throw new ResponseIOException(new SocketException( - "Connection reset by peer: socket write error")); - } + response.reset(); - @Override - public void write(CharSequence sequence) + fail(); + } + catch (IllegalStateException expected) { - throw new ResponseIOException(new SocketException( - "Connection reset by peer: socket write error")); } } } http://git-wip-us.apache.org/repos/asf/wicket/blob/7b711dcd/wicket-core/src/test/java/org/apache/wicket/protocol/http/ResponseIOExceptionTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/protocol/http/ResponseIOExceptionTest.java b/wicket-core/src/test/java/org/apache/wicket/protocol/http/ResponseIOExceptionTest.java new file mode 100644 index 0000000..426d209 --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/protocol/http/ResponseIOExceptionTest.java @@ -0,0 +1,134 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.protocol.http; + +import java.net.SocketException; + +import javax.servlet.http.HttpServletResponse; + +import org.apache.wicket.protocol.http.servlet.ResponseIOException; +import org.apache.wicket.protocol.http.servlet.ServletWebRequest; +import org.apache.wicket.protocol.http.servlet.ServletWebResponse; +import org.apache.wicket.request.IRequestHandler; +import org.apache.wicket.request.Response; +import org.apache.wicket.request.cycle.AbstractRequestCycleListener; +import org.apache.wicket.request.cycle.RequestCycle; +import org.apache.wicket.request.handler.EmptyRequestHandler; +import org.apache.wicket.request.resource.ResourceStreamResource; +import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTester; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + + +/** + * @author Pedro Santos + */ +public class ResponseIOExceptionTest extends Assert +{ + private WicketTester tester; + + /** + * @throws Exception + */ + @Before + public void before() throws Exception + { + tester = new WicketTester() + { + @Override + protected Response newServletWebResponse(ServletWebRequest servletWebRequest) + { + return new ProblematicResponse(servletWebRequest, getResponse()); + } + }; + tester.setExposeExceptions(false); + } + + /** + * @throws Exception + */ + @After + public void after() throws Exception + { + tester.destroy(); + } + + /** + * WICKET-3570 + */ + @Test + public void giveUpRespondingOnIOExceptions() + { + TestRequestCycleListener testRequestCycleListener = new TestRequestCycleListener(); + tester.getApplication().getRequestCycleListeners().add(testRequestCycleListener); + tester.startResource(new ResourceStreamResource(new StringResourceStream("asdf"))); + assertTrue(testRequestCycleListener.lastExceptionRquestHandlerResolved instanceof EmptyRequestHandler); + } + + class TestRequestCycleListener extends AbstractRequestCycleListener + { + IRequestHandler lastExceptionRquestHandlerResolved; + + @Override + public void onExceptionRequestHandlerResolved(RequestCycle cycle, IRequestHandler handler, + Exception exception) + { + lastExceptionRquestHandlerResolved = handler; + } + + } + /** + * Mock response simulating connection lost problems. + */ + public static class ProblematicResponse extends ServletWebResponse + { + + /** + * @param webRequest + * @param httpServletResponse + */ + public ProblematicResponse(ServletWebRequest webRequest, + HttpServletResponse httpServletResponse) + { + super(webRequest, httpServletResponse); + } + + @Override + public void flush() + { + throw new ResponseIOException(new SocketException( + "Connection reset by peer: socket write error")); + } + + @Override + public void write(byte[] array) + { + throw new ResponseIOException(new SocketException( + "Connection reset by peer: socket write error")); + } + + @Override + public void write(CharSequence sequence) + { + throw new ResponseIOException(new SocketException( + "Connection reset by peer: socket write error")); + } + } +} http://git-wip-us.apache.org/repos/asf/wicket/blob/7b711dcd/wicket-request/src/main/java/org/apache/wicket/request/HttpHeaderCollection.java ---------------------------------------------------------------------- diff --git a/wicket-request/src/main/java/org/apache/wicket/request/HttpHeaderCollection.java b/wicket-request/src/main/java/org/apache/wicket/request/HttpHeaderCollection.java index 8e0cb55..e7851e8 100644 --- a/wicket-request/src/main/java/org/apache/wicket/request/HttpHeaderCollection.java +++ b/wicket-request/src/main/java/org/apache/wicket/request/HttpHeaderCollection.java @@ -32,9 +32,9 @@ import org.apache.wicket.util.time.Time; /** * a multivalue map of headers names and header values suitable for * processing http request and response headers. - * + * * @author Peter Ertl - * + * * @since 1.5 */ public class HttpHeaderCollection @@ -51,11 +51,11 @@ public class HttpHeaderCollection /** * internally add new object to header values - * + * * @param name - * header name + * header name * @param object - * header value (can be a string or a {@link Time} object + * header value (can be a string or a {@link Time} object */ private void internalAdd(String name, Object object) { @@ -73,11 +73,11 @@ public class HttpHeaderCollection /** * set header value (and remove previous values) - * + * * @param name - * header name + * header name * @param value - * header value + * header value */ public void setHeader(String name, String value) { @@ -90,11 +90,11 @@ public class HttpHeaderCollection /** * add header value - * + * * @param name - * header name + * header name * @param value - * header value + * header value */ public void addHeader(String name, String value) { @@ -106,11 +106,11 @@ public class HttpHeaderCollection /** * add date header value - * + * * @param name - * header name + * header name * @param time - * timestamp + * timestamp */ public void addDateHeader(String name, Time time) { @@ -119,11 +119,11 @@ public class HttpHeaderCollection /** * add date header value - * + * * @param name - * header name + * header name * @param time - * timestamp + * timestamp */ public void setDateHeader(String name, Time time) { @@ -136,9 +136,9 @@ public class HttpHeaderCollection /** * remove header values for header name - * + * * @param name - * header name + * header name */ public void removeHeader(String name) { @@ -170,9 +170,9 @@ public class HttpHeaderCollection /** * check if header is defined - * + * * @param name - * header name + * header name * @return <code>true</code> if header has one or more values */ public boolean containsHeader(String name) @@ -192,7 +192,7 @@ public class HttpHeaderCollection /** * returns names of headers - * + * * @return set of header names */ public Set<String> getHeaderNames() @@ -213,10 +213,10 @@ public class HttpHeaderCollection /** * get header values (dates will be converted into strings) - * + * * @param name - * header name - * + * header name + * * @return array of header values or empty array if not found */ public String[] getHeaderValues(String name) @@ -241,7 +241,7 @@ public class HttpHeaderCollection { final List<Object> objects = headers.get(new HeaderKey(name)); - if (objects.isEmpty()) + if (objects == null || objects.isEmpty()) { return null; } @@ -267,7 +267,7 @@ public class HttpHeaderCollection /** * check if collection is empty - * + * * @return <code>true</code> if collection is empty, <code>false</code> otherwise */ public boolean isEmpty() @@ -277,7 +277,7 @@ public class HttpHeaderCollection /** * get number of headers - * + * * @return count */ public int getCount()
