Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deepthi M|Updated to show details of error message only when a global property is enabled,Fixed failing test cases and added new tests for the updated method. #591

Merged
merged 6 commits into from
Sep 25, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,12 @@ public class RestConstants {
* Constants used for the Server Log REST Service privilege checking
*/
public static final String PRIV_GET_SERVER_LOGS = "Get Server Logs";
/**
* Global property name used to enable or disable the inclusion of stack trace details
* in the error response.
*
* When this property is set to 'true', stack trace details will be included in error
* responses. When set to 'false', stack trace details will be omitted.
*/
public static String ENABLE_STACK_TRACE_DETAILS_GLOBAL_PROPERTY_NAME = MODULE_ID + ".enableStackTraceDetails";
}
Original file line number Diff line number Diff line change
Expand Up @@ -828,16 +828,21 @@ public static SimpleObject wrapErrorResponse(Exception ex, String reason) {
} else {
map.put("message", "[" + message + "]");
}
StackTraceElement[] steElements = ex.getStackTrace();
if (steElements.length > 0) {
StackTraceElement ste = ex.getStackTrace()[0];
map.put("code", ste.getClassName() + ":" + ste.getLineNumber());
map.put("detail", ExceptionUtils.getStackTrace(ex));
StackTraceElement[] stackTraceElements = ex.getStackTrace();
if (stackTraceElements.length > 0) {
StackTraceElement stackTraceElement = ex.getStackTrace()[0];
String stackTraceDetailsEnabledGp = Context.getAdministrationService()
.getGlobalPropertyValue(RestConstants.ENABLE_STACK_TRACE_DETAILS_GLOBAL_PROPERTY_NAME, "false");
map.put("code", stackTraceElement.getClassName() + ":" + stackTraceElement.getLineNumber());
if ("true".equalsIgnoreCase(stackTraceDetailsEnabledGp)) {
map.put("detail", ExceptionUtils.getStackTrace(ex));
} else {
map.put("detail", "");
}
} else {
map.put("code", "");
map.put("detail", "");
}

return new SimpleObject().add("error", map);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@
import org.junit.Assert;
import org.junit.Test;
import org.mockito.Mockito;
import org.openmrs.GlobalProperty;
import org.openmrs.api.context.Context;
import org.openmrs.module.webservices.rest.SimpleObject;
import org.openmrs.web.test.BaseModuleWebContextSensitiveTest;
import org.springframework.mock.web.MockHttpServletRequest;

/**
* Tests for the {@link RestUtil} class.
*/
public class RestUtilTest {
public class RestUtilTest extends BaseModuleWebContextSensitiveTest {

/**
* @see RestUtil#ipMatches(String,List)
Expand Down Expand Up @@ -201,5 +204,24 @@ public void wrapErrorResponse_shouldSetStackTraceCodeAndDetailEmptyIfNotAvailabl
Assert.assertEquals("", errorResponseMap.get("code"));
Assert.assertEquals("", errorResponseMap.get("detail"));
}

@Test
public void wrapErrorResponse_shouldSetStackTraceDetailsIfGlobalPropEnabled() throws Exception {
Context.getAdministrationService().saveGlobalProperty(
new GlobalProperty(RestConstants.ENABLE_STACK_TRACE_DETAILS_GLOBAL_PROPERTY_NAME, "true"));
Exception ex = new Exception("exceptionmessage");
SimpleObject returnObject = RestUtil.wrapErrorResponse(ex, "wraperrorresponsemessage");

LinkedHashMap errorResponseMap = (LinkedHashMap) returnObject.get("error");
Assert.assertNotEquals("", errorResponseMap.get("detail"));
}
@Test
public void wrapErrorResponse_shouldSetNoStackTraceDetailsIfGlobalPropDisabled() throws Exception {
Context.getAdministrationService().saveGlobalProperty(
new GlobalProperty(RestConstants.ENABLE_STACK_TRACE_DETAILS_GLOBAL_PROPERTY_NAME, "false"));
Exception ex = new Exception("exceptionmessage");
SimpleObject returnObject = RestUtil.wrapErrorResponse(ex, "wraperrorresponsemessage");

LinkedHashMap errorResponseMap = (LinkedHashMap) returnObject.get("error");
Assert.assertEquals("", errorResponseMap.get("detail"));
}
}
5 changes: 5 additions & 0 deletions omod/src/main/resources/config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@
<defaultValue>true</defaultValue>
<description>If the value of this setting is "true", then nothing is logged while the Swagger specification is being generated.</description>
</globalProperty>
<globalProperty>
<property>@[email protected]</property>
<defaultValue>true</defaultValue>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually put space after commas.

<description>If the value of this setting is "true", then the details of the stackTrace would be shown in the error response. However, the recommendation is to keep it as "false", from the Security perspective, to avoid leaking implementation details.</description>
</globalProperty>

<!-- DWR -->

Expand Down
Loading