Skip to content

Commit fc4fd1e

Browse files
jeremiahjstaceyjeremiahjstacey
committed
LogPrefixAppender output enhancements
Making the implementation smarter in order to limit the prefix content to only pertient content based on whether the delegate suppliers are returning valid loggable information.
1 parent 1d9a24a commit fc4fd1e

File tree

2 files changed

+166
-98
lines changed

2 files changed

+166
-98
lines changed

‎src/main/java/org/owasp/esapi/logging/appender/LogPrefixAppender.java‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
*/
2424
public class LogPrefixAppender implements LogAppender {
2525
/** Output format used to assemble return values. */
26-
private static final String RESULT_FORMAT = "[%s %s:%s -> %s] %s";// EVENT_TYPE, CLIENT_INFO, SERVER_INFO, messageBody
26+
private static final String RESULT_FORMAT = "[%s] %s"; //Assembled Prefix, MSG
2727

2828
/** Whether or not to record user information. */
2929
private final boolean logUserInfo;
@@ -63,15 +63,34 @@ public String appendTo(String logName, EventType eventType, String message) {
6363
ClientInfoSupplier clientInfoSupplier = new ClientInfoSupplier();
6464
clientInfoSupplier.setLogClientInfo(logClientInfo);
6565

66-
ServerInfoSupplier serverInfoSupplier = new ServerInfoSupplier(logName);
66+
ServerInfoSupplier serverInfoSupplier = new ServerInfoSupplier(logName);
6767
serverInfoSupplier.setLogServerIp(logServerIp);
6868
serverInfoSupplier.setLogApplicationName(logApplicationName, appName);
6969

70-
String eventTypeMsg = eventTypeSupplier.get();
71-
String userInfoMsg = userInfoSupplier.get();
72-
String clientInfoMsg = clientInfoSupplier.get();
73-
String serverInfoMsg = serverInfoSupplier.get();
70+
String eventTypeMsg = eventTypeSupplier.get().trim();
71+
String userInfoMsg = userInfoSupplier.get().trim();
72+
String clientInfoMsg = clientInfoSupplier.get().trim();
73+
String serverInfoMsg = serverInfoSupplier.get().trim();
74+
75+
//If both user and client have content, then postfix the semicolon to the userInfoMsg at this point to simplify the StringBuilder operations later.
76+
userInfoMsg = (!userInfoMsg.isEmpty() && !clientInfoMsg.isEmpty()) ? userInfoMsg + ":" : userInfoMsg;
77+
78+
//If both server has content, then prefix the arrow to the serverInfoMsg at this point to simplify the StringBuilder operations later.
79+
serverInfoMsg = (!serverInfoMsg.isEmpty()) ? "-> " + serverInfoMsg: serverInfoMsg;
80+
81+
String[] optionalPrefixContent = new String[] {userInfoMsg + clientInfoMsg, serverInfoMsg};
7482

75-
return String.format(RESULT_FORMAT, eventTypeMsg, userInfoMsg, clientInfoMsg, serverInfoMsg, message);
83+
StringBuilder logPrefix = new StringBuilder();
84+
//EventType is always appended
85+
logPrefix.append(eventTypeMsg);
86+
87+
for (String element : optionalPrefixContent) {
88+
if (!element.isEmpty()) {
89+
logPrefix.append(" ");
90+
logPrefix.append(element);
91+
}
92+
}
93+
94+
return String.format(RESULT_FORMAT, logPrefix.toString(), message);
7695
}
7796
}

‎src/test/java/org/owasp/esapi/logging/appender/LogPrefixAppenderTest.java‎

Lines changed: 140 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import static org.mockito.Mockito.spy;
55
import static org.mockito.Mockito.times;
66
import static org.mockito.Mockito.verify;
7-
import static org.mockito.Mockito.verifyNoMoreInteractions;
87
import static org.mockito.Mockito.when;
98
import static org.powermock.api.mockito.PowerMockito.whenNew;
109

@@ -22,96 +21,146 @@
2221
@RunWith(PowerMockRunner.class)
2322
@PrepareForTest(LogPrefixAppender.class)
2423
public class LogPrefixAppenderTest {
25-
@Rule
26-
public TestName testName = new TestName();
27-
28-
private EventTypeLogSupplier etlsSpy;
29-
private String etlsSpyGet = "EVENT_TYPE";
30-
31-
private ClientInfoSupplier cisSpy;
32-
private String cisSpyGet = "CLIENT_INFO";
24+
private static final String EMPTY_RESULT = " ";
25+
private static final String ETL_RESULT = "EVENT_TYPE";
26+
private static final String CIS_RESULT = "CLIENT_INFO";
27+
private static final String UIS_RESULT = "USER_INFO";
28+
private static final String SIS_RESULT = "SERVER_INFO";
29+
30+
@Rule
31+
public TestName testName = new TestName();
3332

34-
private UserInfoSupplier uisSpy;
35-
private String uisSpyGet = "USER_INFO";
36-
37-
private ServerInfoSupplier sisSpy;
38-
private String sisSpyGet = "SERVER_INFO";
39-
40-
@Before
41-
public void buildSupplierSpies() {
42-
etlsSpy = spy(new EventTypeLogSupplier(Logger.EVENT_UNSPECIFIED));
43-
uisSpy = spy(new UserInfoSupplier());
44-
cisSpy = spy(new ClientInfoSupplier());
45-
sisSpy = spy(new ServerInfoSupplier(testName.getMethodName()));
46-
47-
when(etlsSpy.get()).thenReturn(etlsSpyGet);
48-
when(uisSpy.get()).thenReturn(uisSpyGet);
49-
when(cisSpy.get()).thenReturn(cisSpyGet);
50-
when(sisSpy.get()).thenReturn(sisSpyGet);
51-
}
52-
53-
@Test
54-
public void verifyDelegatePassthroughCreation() throws Exception {
55-
ArgumentCaptor<EventType> eventTypeCapture = ArgumentCaptor.forClass(EventType.class);
56-
ArgumentCaptor<String> logNameCapture = ArgumentCaptor.forClass(String.class);
57-
whenNew(EventTypeLogSupplier.class).withArguments(eventTypeCapture.capture()).thenReturn(etlsSpy);
58-
whenNew(UserInfoSupplier.class).withNoArguments().thenReturn(uisSpy);
59-
whenNew(ClientInfoSupplier.class).withNoArguments().thenReturn(cisSpy);
60-
whenNew(ServerInfoSupplier.class).withArguments(logNameCapture.capture()).thenReturn(sisSpy);
61-
62-
LogPrefixAppender lpa = new LogPrefixAppender(true, true, true, true, testName.getMethodName() + "-APPLICATION");
63-
String result = lpa.appendTo(testName.getMethodName() + "-LOGGER", Logger.EVENT_UNSPECIFIED,
64-
testName.getMethodName() + "-MESSAGE");
65-
66-
// Based on the forced returns in the before block
67-
assertEquals("[EVENT_TYPE USER_INFO:CLIENT_INFO -> SERVER_INFO] " + testName.getMethodName() + "-MESSAGE", result);
68-
69-
assertEquals(Logger.EVENT_UNSPECIFIED, eventTypeCapture.getValue());
70-
assertEquals(testName.getMethodName() + "-LOGGER", logNameCapture.getValue());
71-
72-
verify(etlsSpy, times(1)).get();
73-
verify(uisSpy, times(1)).get();
74-
verify(cisSpy, times(1)).get();
75-
verify(sisSpy, times(1)).get();
76-
77-
verify(uisSpy, times(1)).setLogUserInfo(true);
78-
verify(cisSpy, times(1)).setLogClientInfo(true);
79-
verify(sisSpy, times(1)).setLogServerIp(true);
80-
verify(sisSpy, times(1)).setLogApplicationName(true, testName.getMethodName() + "-APPLICATION");
81-
82-
verifyNoMoreInteractions(etlsSpy, uisSpy, cisSpy, sisSpy);
83-
}
84-
85-
@Test
86-
public void verifyDelegatePassthroughCreation2() throws Exception {
87-
ArgumentCaptor<EventType> eventTypeCapture = ArgumentCaptor.forClass(EventType.class);
88-
ArgumentCaptor<String> logNameCapture = ArgumentCaptor.forClass(String.class);
89-
whenNew(EventTypeLogSupplier.class).withArguments(eventTypeCapture.capture()).thenReturn(etlsSpy);
90-
whenNew(UserInfoSupplier.class).withNoArguments().thenReturn(uisSpy);
91-
whenNew(ClientInfoSupplier.class).withNoArguments().thenReturn(cisSpy);
92-
whenNew(ServerInfoSupplier.class).withArguments(logNameCapture.capture()).thenReturn(sisSpy);
93-
94-
LogPrefixAppender lpa = new LogPrefixAppender(false, false, false, false, null);
95-
String result = lpa.appendTo(testName.getMethodName() + "-LOGGER", Logger.EVENT_UNSPECIFIED,
96-
testName.getMethodName() + "-MESSAGE");
97-
98-
// Based on the forced returns in the before block
99-
assertEquals("[EVENT_TYPE USER_INFO:CLIENT_INFO -> SERVER_INFO] " + testName.getMethodName() + "-MESSAGE", result);
100-
101-
assertEquals(Logger.EVENT_UNSPECIFIED, eventTypeCapture.getValue());
102-
assertEquals(testName.getMethodName() + "-LOGGER", logNameCapture.getValue());
103-
104-
verify(etlsSpy, times(1)).get();
105-
verify(uisSpy, times(1)).get();
106-
verify(cisSpy, times(1)).get();
107-
verify(sisSpy, times(1)).get();
108-
109-
verify(uisSpy, times(1)).setLogUserInfo(false);
110-
verify(cisSpy, times(1)).setLogClientInfo(false);
111-
verify(sisSpy, times(1)).setLogServerIp(false);
112-
verify(sisSpy, times(1)).setLogApplicationName(false, null);
113-
114-
verifyNoMoreInteractions(etlsSpy, uisSpy, cisSpy, sisSpy);
115-
}
33+
private String testLoggerName = testName.getMethodName() + "-LOGGER";
34+
private String testLogMessage = testName.getMethodName() + "-MESSAGE";
35+
private String testApplicationName = testName.getMethodName() + "-APPLICATION_NAME";
36+
private EventType testEventType = Logger.EVENT_UNSPECIFIED;
11637

38+
private EventTypeLogSupplier etlsSpy;
39+
private ClientInfoSupplier cisSpy;
40+
private UserInfoSupplier uisSpy;
41+
private ServerInfoSupplier sisSpy;
42+
43+
@Before
44+
public void buildSupplierSpies() {
45+
etlsSpy = spy(new EventTypeLogSupplier(Logger.EVENT_UNSPECIFIED));
46+
uisSpy = spy(new UserInfoSupplier());
47+
cisSpy = spy(new ClientInfoSupplier());
48+
sisSpy = spy(new ServerInfoSupplier(testName.getMethodName()));
49+
50+
testLoggerName = testName.getMethodName() + "-LOGGER";
51+
testLogMessage = testName.getMethodName() + "-MESSAGE";
52+
testApplicationName = testName.getMethodName() + "-APPLICATION_NAME";
53+
}
54+
@Test
55+
public void testCtrArgTruePassthroughToDelegates() throws Exception {
56+
when(etlsSpy.get()).thenReturn(ETL_RESULT);
57+
when(uisSpy.get()).thenReturn(UIS_RESULT);
58+
when(cisSpy.get()).thenReturn(CIS_RESULT);
59+
when(sisSpy.get()).thenReturn(SIS_RESULT);
60+
61+
whenNew(EventTypeLogSupplier.class).withArguments(testEventType).thenReturn(etlsSpy);
62+
whenNew(UserInfoSupplier.class).withNoArguments().thenReturn(uisSpy);
63+
whenNew(ClientInfoSupplier.class).withNoArguments().thenReturn(cisSpy);
64+
whenNew(ServerInfoSupplier.class).withArguments(testLoggerName).thenReturn(sisSpy);
65+
66+
LogPrefixAppender lpa = new LogPrefixAppender(true, true,true,true, testApplicationName);
67+
lpa.appendTo(testLoggerName, testEventType, testLogMessage);
68+
69+
verify(uisSpy, times(1)).setLogUserInfo(true);
70+
verify(cisSpy, times(1)).setLogClientInfo(true);
71+
verify(sisSpy, times(1)).setLogServerIp(true);
72+
verify(sisSpy, times(1)).setLogApplicationName(true, testApplicationName);
73+
}
74+
75+
@Test
76+
public void testCtrArgFalsePassthroughToDelegates() throws Exception {
77+
when(etlsSpy.get()).thenReturn(ETL_RESULT);
78+
when(uisSpy.get()).thenReturn(UIS_RESULT);
79+
when(cisSpy.get()).thenReturn(CIS_RESULT);
80+
when(sisSpy.get()).thenReturn(SIS_RESULT);
81+
82+
whenNew(EventTypeLogSupplier.class).withArguments(testEventType).thenReturn(etlsSpy);
83+
whenNew(UserInfoSupplier.class).withNoArguments().thenReturn(uisSpy);
84+
whenNew(ClientInfoSupplier.class).withNoArguments().thenReturn(cisSpy);
85+
whenNew(ServerInfoSupplier.class).withArguments(testLoggerName).thenReturn(sisSpy);
86+
87+
LogPrefixAppender lpa = new LogPrefixAppender(false, false, false, false, null);
88+
lpa.appendTo(testLoggerName, testEventType, testLogMessage);
89+
90+
verify(uisSpy, times(1)).setLogUserInfo(false);
91+
verify(cisSpy, times(1)).setLogClientInfo(false);
92+
verify(sisSpy, times(1)).setLogServerIp(false);
93+
verify(sisSpy, times(1)).setLogApplicationName(false, null);
94+
}
95+
96+
@Test
97+
public void testDelegateCtrArgs() throws Exception {
98+
ArgumentCaptor<EventType> eventTypeCapture = ArgumentCaptor.forClass(EventType.class);
99+
ArgumentCaptor<String> logNameCapture = ArgumentCaptor.forClass(String.class);
100+
whenNew(EventTypeLogSupplier.class).withArguments(eventTypeCapture.capture()).thenReturn(etlsSpy);
101+
whenNew(UserInfoSupplier.class).withNoArguments().thenReturn(uisSpy);
102+
whenNew(ClientInfoSupplier.class).withNoArguments().thenReturn(cisSpy);
103+
whenNew(ServerInfoSupplier.class).withArguments(logNameCapture.capture()).thenReturn(sisSpy);
104+
105+
LogPrefixAppender lpa = new LogPrefixAppender(true, true,true,true, testApplicationName);
106+
lpa.appendTo(testLoggerName, testEventType, testLogMessage);
107+
108+
assertEquals(testEventType, eventTypeCapture.getValue());
109+
assertEquals(testLoggerName, logNameCapture.getValue());
110+
}
111+
112+
@Test
113+
public void testLogContentWhenClientInfoEmpty() throws Exception {
114+
runTest(ETL_RESULT, UIS_RESULT, EMPTY_RESULT,SIS_RESULT, "[EVENT_TYPE USER_INFO -> SERVER_INFO]");
115+
}
116+
117+
118+
@Test
119+
public void testLogContentWhenUserInfoEmpty() throws Exception {
120+
runTest(ETL_RESULT, EMPTY_RESULT, CIS_RESULT,SIS_RESULT, "[EVENT_TYPE CLIENT_INFO -> SERVER_INFO]");
121+
}
122+
123+
@Test
124+
public void testLogContentWhenClientInfoEmptyAndServerInfoEmpty() throws Exception {
125+
runTest(ETL_RESULT, UIS_RESULT, EMPTY_RESULT,EMPTY_RESULT, "[EVENT_TYPE USER_INFO]");
126+
}
127+
128+
@Test
129+
public void testLogContentWhenUserInfoEmptyAndServerInfoEmpty() throws Exception {
130+
runTest(ETL_RESULT, EMPTY_RESULT, CIS_RESULT,EMPTY_RESULT, "[EVENT_TYPE CLIENT_INFO]");
131+
}
132+
133+
@Test
134+
public void testLogContentWhenUserInfoAndClientInfoEmpty() throws Exception {
135+
runTest(ETL_RESULT, EMPTY_RESULT, EMPTY_RESULT, SIS_RESULT, "[EVENT_TYPE -> SERVER_INFO]");
136+
}
137+
138+
@Test
139+
public void testLogContentWhenServerInfoEmpty() throws Exception {
140+
runTest(ETL_RESULT, UIS_RESULT, CIS_RESULT, EMPTY_RESULT, "[EVENT_TYPE USER_INFO:CLIENT_INFO]");
141+
}
142+
143+
@Test
144+
public void testLogContentWhenUserInfoEmptyAndClientInfoEmptyAndServerInfoEmpty() throws Exception {
145+
runTest(ETL_RESULT, EMPTY_RESULT, EMPTY_RESULT, EMPTY_RESULT, "[EVENT_TYPE]");
146+
}
147+
148+
149+
private void runTest(String typeResult, String userResult, String clientResult, String serverResult, String exResult) throws Exception{
150+
when(etlsSpy.get()).thenReturn(typeResult);
151+
when(uisSpy.get()).thenReturn(userResult);
152+
when(cisSpy.get()).thenReturn(clientResult);
153+
when(sisSpy.get()).thenReturn(serverResult);
154+
155+
whenNew(EventTypeLogSupplier.class).withArguments(testEventType).thenReturn(etlsSpy);
156+
whenNew(UserInfoSupplier.class).withNoArguments().thenReturn(uisSpy);
157+
whenNew(ClientInfoSupplier.class).withNoArguments().thenReturn(cisSpy);
158+
whenNew(ServerInfoSupplier.class).withArguments(testLoggerName).thenReturn(sisSpy);
159+
160+
//Since everything is mocked these booleans don't much matter aside from the later verifies
161+
LogPrefixAppender lpa = new LogPrefixAppender(false, false, false, false, null);
162+
String result = lpa.appendTo(testLoggerName, testEventType, testLogMessage);
163+
164+
assertEquals(exResult + " " + testName.getMethodName() + "-MESSAGE", result);
165+
}
117166
}

0 commit comments

Comments
 (0)