Updated Branches: refs/heads/master 1f5989f54 -> 8c827e33e
WICKET-4927 allow headers to be set even if meta data is no longer buffered Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/8c827e33 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/8c827e33 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/8c827e33 Branch: refs/heads/master Commit: 8c827e33e6ae1d4177df2eab9bd3cf9b5a84248f Parents: d70b7f4 Author: svenmeier <[email protected]> Authored: Fri Dec 14 16:15:04 2012 +0100 Committer: svenmeier <[email protected]> Committed: Fri Dec 14 16:18:19 2012 +0100 ---------------------------------------------------------------------- .../protocol/http/HeaderBufferingWebResponse.java | 98 ++++++++------- .../http/HeaderBufferingWebResponseTest.java | 82 ++++++++++++ 2 files changed, 134 insertions(+), 46 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/8c827e33/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 0ac0c2a..8771dca 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; } @Override http://git-wip-us.apache.org/repos/asf/wicket/blob/8c827e33/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 new file mode 100644 index 0000000..1383b4b --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/protocol/http/HeaderBufferingWebResponseTest.java @@ -0,0 +1,82 @@ +/* + * 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 org.apache.wicket.mock.MockWebResponse; +import org.junit.Assert; +import org.junit.Test; + + +/** + * Test for {@link HeaderBufferingWebResponse}. + * + * @author svenmeier + */ +public class HeaderBufferingWebResponseTest extends Assert +{ + + /** + * WICKET-4927 + */ + @Test + public void additionalHeaderAfterWrittenContent() + { + MockWebResponse originalResponse = new MockWebResponse(); + + 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")); + } + + /** + */ + @Test + public void resetAfterWrittenContent() + { + MockWebResponse originalResponse = new MockWebResponse(); + + HeaderBufferingWebResponse response = new HeaderBufferingWebResponse(originalResponse); + + response.addHeader("key1", "value1"); + + assertNull(originalResponse.getHeader("key1")); + + response.reset(); + + response.write("written"); + + try + { + response.reset(); + + fail(); + } + catch (IllegalStateException expected) + { + } + } +}
