Bug 562440 - method getStatus() of class HttpServletResponseWrapperImpl
returns -1 in case of success
Change-Id: Iaf063c71e2f62e7526ddfe5b75ee5637b687b7fd
Signed-off-by: Anjum Fatima <anjum.eclipse@gmail.com>
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 cb4c789..04d624f 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 @@
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 @@
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 0000000..5308705
--- /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 453ad66..9fc74c6 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 @@
}
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 ede5cef..07b8cc3 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 @@
@Override
public int getStatus() {
+ if (status == -1) {
+ return super.getStatus();
+ }
+ return status;
+ }
+
+ public int getInternalStatus() {
return status;
}
@@ -67,6 +74,15 @@
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 @@
@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 6c2c88d..149375a 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 @@
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 @@
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;
}
