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

Fix Tee Filter #413

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ecs-logging-access/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ dependencies {
implementation("ch.qos.logback.access:tomcat:2.0.3"){
exclude group: 'org.apache.tomcat'
}
implementation('org.apache.commons:commons-io:1.3.2')
compileOnly("org.springframework.boot:spring-boot-starter-web")
compileOnly("org.springframework.boot:spring-boot-starter-webflux")

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.raynigon.ecs.logging.access.server;

import ch.qos.logback.access.common.servlet.TeeFilter;
import com.raynigon.ecs.logging.access.AccessLogProperties;
import org.springframework.boot.autoconfigure.AutoConfiguration;
import org.springframework.boot.autoconfigure.AutoConfigureBefore;
Expand Down Expand Up @@ -28,7 +27,7 @@ public class AccessLogFilterConfiguration {
@Bean
@NonNull
@ConditionalOnProperty(value = "raynigon.logging.access.export-body", havingValue = "true")
public FilterRegistrationBean<TeeFilter> requestLoggingFilter() {
return new FilterRegistrationBean<>(new TeeFilter());
public FilterRegistrationBean<CustomTeeFilter> requestLoggingFilter() {
return new FilterRegistrationBean<>(new CustomTeeFilter());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package com.raynigon.ecs.logging.access.server;

import ch.qos.logback.access.common.AccessConstants;
import jakarta.servlet.*;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

import java.io.IOException;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.List;

public class CustomTeeFilter implements Filter {
boolean active;

@Override
public void destroy() {
// NOP
}

@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain)
throws IOException, ServletException {

if (active && request instanceof HttpServletRequest) {
try {
TeeHttpServletRequest teeRequest = new TeeHttpServletRequest((HttpServletRequest) request);
TeeHttpServletResponse teeResponse = new TeeHttpServletResponse((HttpServletResponse) response);

filterChain.doFilter(teeRequest, teeResponse);

teeResponse.finish();
// let the output contents be available for later use by
// logback-access-logging
teeRequest.setAttribute(AccessConstants.LB_OUTPUT_BUFFER, teeResponse.getOutputBuffer());
} catch (IOException e) {
e.printStackTrace();
throw e;
} catch (ServletException e) {
e.printStackTrace();
throw e;
}
} else {
filterChain.doFilter(request, response);
}

}

@Override
public void init(FilterConfig filterConfig) throws ServletException {
String includeListAsStr = filterConfig.getInitParameter(AccessConstants.TEE_FILTER_INCLUDES_PARAM);
String excludeListAsStr = filterConfig.getInitParameter(AccessConstants.TEE_FILTER_EXCLUDES_PARAM);
String localhostName = getLocalhostName();

active = computeActivation(localhostName, includeListAsStr, excludeListAsStr);
// Log the activation status on stdout, because loggers cannot be used here
if (active) {
System.out.println("TeeFilter will be ACTIVE on this host [" + localhostName + "]");

Choose a reason for hiding this comment

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

Since this is a logging library, it seems reasonable to use a logger instead.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Wont work, because its a logging library...
I will remove all the print statements.

} else {
System.out.println("TeeFilter will be DISABLED on this host [" + localhostName + "]");
}
}

public static List<String> extractNameList(String nameListAsStr) {
List<String> nameList = new ArrayList<String>();
if (nameListAsStr == null) {
return nameList;
}

nameListAsStr = nameListAsStr.trim();

Check warning on line 71 in ecs-logging-access/src/main/java/com/raynigon/ecs/logging/access/server/CustomTeeFilter.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

ecs-logging-access/src/main/java/com/raynigon/ecs/logging/access/server/CustomTeeFilter.java#L71

Avoid reassigning parameters such as 'nameListAsStr'
if (nameListAsStr.length() == 0) {
return nameList;
}

String[] nameArray = nameListAsStr.split("[,;]");
for (String n : nameArray) {
n = n.trim();
nameList.add(n);
}
return nameList;
}

static String getLocalhostName() {
String hostname = "127.0.0.1";

try {
hostname = InetAddress.getLocalHost().getHostName();
} catch (UnknownHostException uhe) {
uhe.printStackTrace();
}
return hostname;
}

public static boolean computeActivation(String hostname, String includeListAsStr, String excludeListAsStr) {
List<String> includeList = extractNameList(includeListAsStr);
List<String> excludeList = extractNameList(excludeListAsStr);
boolean inIncludesList = matchesIncludesList(hostname, includeList);
boolean inExcludesList = matchesExcludesList(hostname, excludeList);
return inIncludesList && (!inExcludesList);
}

static boolean matchesIncludesList(String hostname, List<String> includeList) {
if (includeList.isEmpty())
return true;
return includeList.contains(hostname);
}

static boolean matchesExcludesList(String hostname, List<String> excludesList) {
if (excludesList.isEmpty())
return false;
return excludesList.contains(hostname);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package com.raynigon.ecs.logging.access.server;

import ch.qos.logback.access.common.AccessConstants;
import ch.qos.logback.access.common.servlet.Util;
import jakarta.servlet.ServletInputStream;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletRequestWrapper;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class TeeHttpServletRequest extends HttpServletRequestWrapper {

private TeeServletInputStream inStream;
private BufferedReader reader;
boolean postedParametersMode = false;

TeeHttpServletRequest(HttpServletRequest request) {
super(request);
// we can't access the input stream and access the request parameters
// at the same time
if (Util.isFormUrlEncoded(request)) {
postedParametersMode = true;
} else {
inStream = new TeeServletInputStream(request);
// add the contents of the input buffer as an attribute of the request
request.setAttribute(AccessConstants.LB_INPUT_BUFFER, inStream.getInputBuffer());
reader = new BufferedReader(new InputStreamReader(inStream));
}

}

byte[] getInputBuffer() {
if (postedParametersMode) {
throw new IllegalStateException("Call disallowed in postedParametersMode");
}
return inStream.getInputBuffer();
}

@Override
public ServletInputStream getInputStream() throws IOException {
if (!postedParametersMode) {
return inStream;
} else {
return super.getInputStream();
}
}

//

@Override
public BufferedReader getReader() throws IOException {
if (!postedParametersMode) {
return reader;
} else {
return super.getReader();
}
}

public boolean isPostedParametersMode() {
return postedParametersMode;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package com.raynigon.ecs.logging.access.server;


import jakarta.servlet.ServletOutputStream;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpServletResponseWrapper;

import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;

public class TeeHttpServletResponse extends HttpServletResponseWrapper {

TeeServletOutputStream teeServletOutputStream;
PrintWriter teeWriter;

public TeeHttpServletResponse(HttpServletResponse httpServletResponse) {
super(httpServletResponse);
}

@Override
public ServletOutputStream getOutputStream() throws IOException {
if (teeServletOutputStream == null) {
teeServletOutputStream = new TeeServletOutputStream(this.getResponse());
}
return teeServletOutputStream;
}

@Override
public PrintWriter getWriter() throws IOException {
if (this.teeWriter == null) {
this.teeWriter = new PrintWriter(
new OutputStreamWriter(getOutputStream(), this.getResponse().getCharacterEncoding()), true);
}
return this.teeWriter;
}

@Override
public void flushBuffer() {
Copy link
Preview

Copilot AI Nov 12, 2024

Choose a reason for hiding this comment

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

The flushBuffer method should also flush the teeServletOutputStream if it is not null to ensure all data is properly flushed.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
if (this.teeWriter != null) {
this.teeWriter.flush();
}
}

public byte[] getOutputBuffer() {
// teeServletOutputStream can be null if the getOutputStream method is never
// called.
if (teeServletOutputStream != null) {
return teeServletOutputStream.getOutputStreamAsByteArray();
} else {
return null;
}
}

void finish() throws IOException {
if (this.teeWriter != null) {
this.teeWriter.close();
}
if (this.teeServletOutputStream != null) {
this.teeServletOutputStream.close();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package com.raynigon.ecs.logging.access.server;

import jakarta.servlet.ReadListener;
import jakarta.servlet.ServletInputStream;
import jakarta.servlet.http.HttpServletRequest;
import org.apache.commons.io.IOUtils;

import java.io.ByteArrayInputStream;
import java.io.IOException;

public class TeeServletInputStream extends ServletInputStream {

private byte[] content;
private ByteArrayInputStream in;

TeeServletInputStream(HttpServletRequest request) {
duplicateInputStream(request);
}

private void duplicateInputStream(HttpServletRequest request) {
ServletInputStream originalSIS = null;
try {
originalSIS = request.getInputStream();
content = IOUtils.toByteArray(request.getInputStream());
in = new ByteArrayInputStream(content);
} catch (IOException e) {
e.printStackTrace();
Copy link
Preview

Copilot AI Nov 12, 2024

Choose a reason for hiding this comment

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

Catching IOException and calling e.printStackTrace() is not a good practice. Consider proper logging instead.

Suggested change
e.printStackTrace();
LoggerFactory.getLogger(TeeServletInputStream.class).error("Error duplicating input stream", e);

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
} finally {
closeStream(originalSIS);
}
}

@Override
public int read() throws IOException {
return in.read();
}

void closeStream(ServletInputStream is) {
if (is != null) {
try {
is.close();
} catch (IOException e) {
e.printStackTrace();
}
}
}

byte[] getInputBuffer() {
return content;
}

@Override
public boolean isFinished() {
throw new RuntimeException("Not yet implemented");

Check warning on line 54 in ecs-logging-access/src/main/java/com/raynigon/ecs/logging/access/server/TeeServletInputStream.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

ecs-logging-access/src/main/java/com/raynigon/ecs/logging/access/server/TeeServletInputStream.java#L54

Avoid throwing raw exception types.
Copy link
Preview

Copilot AI Nov 12, 2024

Choose a reason for hiding this comment

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

The method isFinished should not throw a RuntimeException. It should be properly implemented or handled gracefully.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
}

@Override
public boolean isReady() {
throw new RuntimeException("Not yet implemented");
Copy link
Preview

Copilot AI Nov 12, 2024

Choose a reason for hiding this comment

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

The method isReady should not throw a RuntimeException. It should be properly implemented or handled gracefully.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
}

@Override
public void setReadListener(ReadListener listener) {
throw new RuntimeException("Not yet implemented");
Copy link
Preview

Copilot AI Nov 12, 2024

Choose a reason for hiding this comment

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

The method setReadListener should not throw a RuntimeException. It should be properly implemented or handled gracefully.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
}
}

Loading