Bug 562440 - method getStatus() of class HttpServletResponseWrapperImplY20200716-0820Y20200715-1200Y20200711-1200Y20200708-1200S4_17_0_M1I20200715-1800I20200714-1800I20200713-2230I20200713-1800I20200712-1800I20200711-2250I20200711-1800I20200710-1800I20200710-0230I20200708-1800I20200708-0600I20200707-1800

returns -1 in case of success

Change-Id: Iaf063c71e2f62e7526ddfe5b75ee5637b687b7fd
Signed-off-by: Anjum Fatima <anjum.eclipse@gmail.com>

5 files changed, 134 insertions, 6 deletions

diff --git a/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/testbase/AllTests.java b/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/testbase/AllTests.java
index cb4c7897d..04d624f7e 100644
--- a/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/testbase/AllTests.java
+++ b/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/testbase/AllTests.java
@@ -15,6 +15,7 @@ package org.eclipse.equinox.http.servlet.testbase;
 
 import org.eclipse.equinox.http.servlet.tests.AuthenticationTest;
 import org.eclipse.equinox.http.servlet.tests.Bug500783_Test;
+import org.eclipse.equinox.http.servlet.tests.Bug562440_Test;
 import org.eclipse.equinox.http.servlet.tests.Bug562843_2_Test;
 import org.eclipse.equinox.http.servlet.tests.Bug562843_Test;
 import org.eclipse.equinox.http.servlet.tests.Bug564747_Test;
@@ -102,7 +103,8 @@ import org.junit.runners.Suite.SuiteClasses;
 	Bug500783_Test.class,
 	Bug562843_Test.class,
 	Bug562843_2_Test.class,
-	Bug564747_Test.class
+	Bug564747_Test.class,
+	Bug562440_Test.class
 })
 public class AllTests {
 	// see @SuiteClasses
diff --git a/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/Bug562440_Test.java b/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/Bug562440_Test.java
new file mode 100644
index 000000000..5308705e7
--- /dev/null
+++ b/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/Bug562440_Test.java
@@ -0,0 +1,105 @@
+/*******************************************************************************
+ * Copyright (c) 2020 IBM Corporation and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.equinox.http.servlet.tests;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.util.Dictionary;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+
+import javax.servlet.Servlet;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.equinox.http.servlet.testbase.BaseTest;
+import org.junit.Test;
+import org.osgi.service.http.whiteboard.HttpWhiteboardConstants;
+
+public class Bug562440_Test extends BaseTest {
+
+	@Test
+	public void test_ServletGetStatus() throws Exception {
+		final AtomicReference<String> status = new AtomicReference<>();
+		final AtomicReference<Boolean> error = new AtomicReference<>(false);
+		Servlet servlet = new HttpServlet() {
+			private static final long serialVersionUID = 1L;
+
+			@Override
+			protected void doGet(
+				HttpServletRequest req, HttpServletResponse resp) throws IOException {
+				if (error.get()) {
+					resp.sendError(404);
+				}
+				status.set(Integer.toString(resp.getStatus()));
+			}
+		};
+
+		Dictionary<String, Object> props = new Hashtable<>();
+		props.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_NAME, "S1");
+		props.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, "/S1/*");
+		registrations.add(getBundleContext().registerService(Servlet.class, servlet, props));
+
+		requestAdvisor.request("S1/a");
+
+		assertEquals("200", status.get());
+
+		error.set(true);
+		try {
+			requestAdvisor.request("S1/a");
+		} catch (IOException e) {
+			assertEquals("404", status.get());
+		}
+	}
+
+	@Test
+	public void test_ServletFlushBuffer() throws Exception {
+		final AtomicReference<Boolean> flushBuffer = new AtomicReference<>(false);
+		Servlet servlet = new HttpServlet() {
+			private static final long serialVersionUID = 1L;
+
+			@Override
+			protected void doGet(
+				HttpServletRequest req, HttpServletResponse resp)
+				throws IOException {
+
+				resp.sendError(404, "NOT FOUND");
+				if (flushBuffer.get()) {
+					resp.flushBuffer();
+				} else {
+					resp.getOutputStream().flush();
+				}
+			}
+		};
+
+		Dictionary<String, Object> props = new Hashtable<>();
+		props.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_NAME, "S1");
+		props.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, "/S1/*");
+		registrations.add(getBundleContext().registerService(Servlet.class, servlet, props));
+
+		Map<String, List<String>> response = requestAdvisor.request("S1/a", null);
+		assertEquals("404", response.get("responseCode").get(0));
+
+		flushBuffer.set(true);
+		response = requestAdvisor.request("S1/a", null);
+		assertEquals("404", response.get("responseCode").get(0));
+		assertTrue(response.get("responseBody").get(0).contains("NOT FOUND"));
+	}
+}
diff --git a/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/util/ServletRequestAdvisor.java b/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/util/ServletRequestAdvisor.java
index 453ad66c9..9fc74c6b8 100644
--- a/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/util/ServletRequestAdvisor.java
+++ b/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/util/ServletRequestAdvisor.java
@@ -235,11 +235,14 @@ public class ServletRequestAdvisor extends Object {
 		}
 
 		try {
-			map.put("responseBody", Arrays.asList(drain(stream)));
-			return map;
+			String drainedStream = drain(stream);
+			map.put("responseBody", Arrays.asList(drainedStream));
+		} catch (IOException e){
+			map.put("responseBody", Arrays.asList(e.getMessage()));
 		} finally {
 			stream.close();
 		}
+		return map;
 	}
 
 	public Map<String, List<String>> eventSource(String value, Map<String, List<String>> headers, final EventHandler handler) throws IOException {
diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpServletResponseWrapperImpl.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpServletResponseWrapperImpl.java
index ede5cefe8..07b8cc320 100644
--- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpServletResponseWrapperImpl.java
+++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpServletResponseWrapperImpl.java
@@ -48,6 +48,13 @@ public class HttpServletResponseWrapperImpl extends HttpServletResponseWrapper {
 
 	@Override
 	public int getStatus() {
+		if (status == -1) {
+			return super.getStatus();
+		}
+		return status;
+	}
+
+	public int getInternalStatus() {
 		return status;
 	}
 
@@ -67,6 +74,15 @@ public class HttpServletResponseWrapperImpl extends HttpServletResponseWrapper {
 		return writer;
 	}
 
+	@Override
+	public void flushBuffer() throws IOException {
+		if (status != -1) {
+			HttpServletResponse wrappedResponse = (HttpServletResponse)this.getResponse();
+			wrappedResponse.sendError(status, getMessage());
+		}
+		super.flushBuffer();
+	}
+
 	public boolean isCompleted() {
 		return completed;
 	}
@@ -108,6 +124,10 @@ public class HttpServletResponseWrapperImpl extends HttpServletResponseWrapper {
 
 		@Override
 		public void flush() throws IOException {
+			if (getInternalStatus() != -1) {
+				HttpServletResponse wrappedResponse = (HttpServletResponse) HttpServletResponseWrapperImpl.this.getResponse();
+				wrappedResponse.sendError(getInternalStatus(), getMessage());
+			}
 			originalOutputStream.flush();
 		}
 
diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ResponseStateHandler.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ResponseStateHandler.java
index 6c2c88dbb..149375a74 100644
--- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ResponseStateHandler.java
+++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ResponseStateHandler.java
@@ -296,7 +296,7 @@ public class ResponseStateHandler {
 			throw new IllegalStateException("Can't locate response impl"); //$NON-NLS-1$
 		}
 
-		final int status = responseWrapper.getStatus();
+		final int status = responseWrapper.getInternalStatus();
 
 		if (status < HttpServletResponse.SC_BAD_REQUEST) {
 			return;
@@ -310,8 +310,6 @@ public class ResponseStateHandler {
 		HttpServletResponse wrappedResponse = (HttpServletResponse)responseWrapper.getResponse();
 
 		if (wrappedResponse.isCommitted()) {
-			// the response is committed already, but we need to propagate the error code anyway
-			wrappedResponse.sendError(status, responseWrapper.getMessage());
 			// There's nothing more we can do here.
 			return;
 		}