Servlet snippet inject not work after call getOutputStream

oliver-zhang commented 1 year ago

Describe the bug I'm using tomcat deploy web, The request after call Servlet3SnippetInjectingResponseWrapper.getOutputStream，snippet inject not work for me I think the reason is the method getOutputSteam is called before method setContentType

What did you expect to see? The script is added to the page

What did you see instead? No change

What version are you using? 1.27

mateuszrzeszutek commented 1 year ago

I think the reason is the method getOutputSteam is called before method setContentType

This is by design -- we can't just inject the snippet into every possible stream unless we are sure that it's an HTML page. Other file types (e.g. binary, XML, or just text files) can also contain the string <head> and if we were to inject the snippet there we would most likely break the application.

cc @trask @siyuniu-ms

oliver-zhang commented 1 year ago

I think the reason is the method getOutputSteam is called before method setContentType

This is by design -- we can't just inject the snippet into every possible stream unless we are sure that it's an HTML page. Other file types (e.g. binary, XML, or just text files) can also contain the string <head> and if we were to inject the snippet there we would most likely break the application.

cc @trask @siyuniu-ms

Actually the stream is an HTML Servlet3SnippetInjectingResponseWrapper.getOutputStreamwill check if the response is html, if not an html nothing will changed, but the method getOutputStream is called before setContentType, so nothing changed even the response content_type will be set to text/html the method getWriter is called after setContentType,so it is no problem can we do it like this?

private ServletOutputStream outputStream;
@Override
public ServletOutputStream getOutputStream() throws IOException {
  if (outputStream == null) {
    outputStream = super.getOutputStream();
  }
  return outputStream;
}
@Override
public void setContentType(String type) {
  super.setContentType(type);
  if (outputStream == null) {
    return;
  }
  initializeInjectionStateIfNeeded(outputStream,this);
}

mateuszrzeszutek commented 1 year ago

Hmm, I think you could make it work that way — but you’d also have to modify the Writer anyway, cause it shares some code with the OutputStream; and make sure that the “is html” condition is checked every time before any byte is processed. Still, if you actually write <head> anything before setting the content-type I'd expect it to fail.

oliver-zhang commented 1 year ago

Hmm, I think you could make it work that way — but you’d also have to modify the Writer anyway, cause it shares some code with the OutputStream; and make sure that the “is html” condition is checked every time before any byte is processed. Still, if you actually write <head> anything before setting the content-type I'd expect it to fail.

Yes, it can check "is html" after setContentType , but how to make sure that the “is html” condition is checked every time before any byte is processed @mateuszrzeszutek

open-telemetry / opentelemetry-java-instrumentation

Servlet snippet inject not work after call getOutputStream #8869