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

"FilterServletResponseWrapper.flushBuffer()" does not flush the response with empty body #836

Closed
dabing009 opened this issue May 15, 2019 · 4 comments

Comments

@dabing009
Copy link

We use javamelody with tomcat on our product environment. We configure javamelody as the first filter for tomcat webapp. Recently we found an issue that tomcat webapp REST API returns 304 without body, but finally user gets 204.

We did some trouble-shooting and found when tomcat webapp call response.flushBuffer(), it ends up with calling javamelody "FilterServletResponseWrapper.flushBuffer()", its code likes below:

 @Override
	public void flushBuffer() throws IOException {
		if (writer != null) {
			writer.flush();
		} else if (stream != null) {
			stream.flush();
		}
    }

Looks it only flush response when the body(either writer or stream) is not empty, for our case, response without body cannot be flushed from here. Then it reaches jersey library, from there it changes the code to 204 due to empty body.

We think it would be better to change the above code to below:

	@Override
	public void flushBuffer() throws IOException {
		if (writer != null) {
			writer.flush();
		} else if (stream != null) {
			stream.flush();
		} else {
			((HttpServletResponse)this.getResponse()).flushBuffer();
		}
	}

If no body found just call the wrapped response flushBuffer() to send out the response status code and headers. With the above changes, our user finally got correct response code 304.

evernat added a commit that referenced this issue May 19, 2019
@evernat
Copy link
Member

evernat commented May 19, 2019

fixed by e0c2fe0 and ready for the next release (1.78).
A snapshot build including the fix is available at:
http://javamelody.org/downloads/javamelody-core-1.78.0-SNAPSHOT.jar

@evernat evernat closed this as completed May 19, 2019
@dabing009
Copy link
Author

Thanks for quick reply!

@evernat
Copy link
Member

evernat commented Jul 24, 2019

The fix of this issue (e0c2fe0) caused JENKINS-58419.
It is not known if the problem is now in javamelody since 1.78.0 or in the jenkins CAS plugin 1.4.3.

@evernat
Copy link
Member

evernat commented Jul 25, 2019

JENKINS-58419 (since monitoring-plugin 1.78) is fixed by 068775a:
MonitoringFilter should not flush buffer when no content.

goldyliang pushed a commit to goldyliang/javamelody that referenced this issue Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants