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

Specify the charset of HTTPServer response #564

Merged
merged 2 commits into from
Aug 11, 2020

Conversation

joaopdelboni
Copy link
Contributor

In some Operational Systems (Like IBM Z/OS and IBM Z/Unix) the JRE runs with a charset different of UTF-8, those default charsets produce wrong behavior and truncated responses.

To avoid this kind of situations I put the specifc charset in the output writer.

This create a consistent response between different platforms.

In some Operational Systems (Like IBM Z/OS and IBM Z/Unix) the JRE runs with a charset different of UTF-8, those default charsets produce wrong behavior and truncated responses.

To avoid this kind of situations I put the specifc charset in the output writer.

This create a consistent response between different platforms.

Signed-off-by: João Paulo Binda Delboni <[email protected]>
@brian-brazil
Copy link
Contributor

Is there anywhere else we need this?

@joaopdelboni
Copy link
Contributor Author

Is there anywhere else we need this?

Only this class for the tested use cases. But this class are used by jmx_exporter (jmx_prometheus_javaagent and jmx_prometheus_httpserver) those projects will be affected by this change.

@brian-brazil
Copy link
Contributor

We've other places we produce HTTP requests/responses with metrics, would you mind checking them?

To prevent problems with encoding in some Operational Systems we set the charset of Graphite Charset.

Signed-off-by: João Paulo Binda Delboni <[email protected]>
@joaopdelboni
Copy link
Contributor Author

@brian-brazil sorry for the delay, I've made a review on the other projects of the repository and thats my view:

  • SpringBoot: The spring boot project made the enconding based on request encoding header. So, we don't need specify this type of detail;
  • VertX: VertX respect the same behavior of spring.
  • Servlet: The metrics servlet endpoint uses the default ServeletResponse buffer, so we don't need to made any type of ajustment.
  • Graphite Bridge: I made a change to include charset on the request. To prevent the same issues related before.
  • Push Gateway: This project specify the enconding, so .. is all right here
  • LogBack, Log4j2, Log4j, Hotspot, SpringWeb, etc..: They don't expose any type of metrics http endpoints.

@brian-brazil
Copy link
Contributor

The spring boot project made the enconding based on request encoding header.

We should always expose UTF-8, no matter what the request headers say.

@joaopdelboni
Copy link
Contributor Author

Ok Brian,

I made a mistake on the previous description, the spring boot have the property named spring.http.encoding.charset, this property have a default value defined to UTF-8.
The only way to spring return something in other encoding is changing this property value.

I can made a change to in the metrics specific endpoint aways return values encoded in UTF-8, this will result in a method with raw bytearray return. This not smell good. But i can do.

What do you think about ?

@brian-brazil
Copy link
Contributor

It sounds like Spring do the right thing by default, so we could leave that until it's actually a problem for someone.

@joaopdelboni
Copy link
Contributor Author

Great, so at this point it is all changes done.

@brian-brazil brian-brazil merged commit 01e1e9e into prometheus:master Aug 11, 2020
@brian-brazil
Copy link
Contributor

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants