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

Document the need to set quarkus.native.add-all-charsets = true in HTTP extensions #599

Closed
ppalaga opened this issue Jan 6, 2020 · 3 comments

Comments

@ppalaga
Copy link
Contributor

ppalaga commented Jan 6, 2020

AHC and HTTP components are the prominent candidates.

Why: They offer the option to decode the response body using the encoding sent by the server which can be any encoding.

#597 (comment)

@ppalaga
Copy link
Contributor Author

ppalaga commented Jan 20, 2020

Thinking of this again, the advantages of producing NativeImageEnableAllCharsetsBuildItem by AHC, HTTP and other extensions would be the following:

  • The JVM mode and native mode would behave more consistently.
  • The behavior probably wanted by the most users would be enabled by default.

An apparent disadvantage would be that the application developer might know that his app is never going to need any encoding beyond the set available by default. This would be the case when the developer has both the client and server under his control. Always producing NativeImageEnableAllCharsetsBuildItem would just make the application unnecessarily bigger.

I am not sure if setting quarkus.native.add-all-charsets = false in the application.properties overrides our default. Even if it does, it would be a pain to document. We'd have to write something like

If you have AHC in your CP, the default of quarkus.native.add-all-charsets is not false like with vanilla Quarkus, but true. If you want to override the override, use quarkus.native.add-all-charsets = false

Maybe to get best of both worlds, we could have our own per-extension props, e.g. quarkus.camel.ahc.enable-all-charsets with default true. It would trigger NativeImageEnableAllCharsetsBuildItem only if it the value is true. So quarkus.camel.ahc.enable-all-charsets = false would work reliably for those few users who want (and know how) to fine tune their app.

Does anybody have an opinion on this?

@lburgazzoli
Copy link
Contributor

Having component specific property is misleading i.e. you can set one it true for one component and fase for another but the result won’t be that one component support all the charset and the other one not.

It looks to me a generic quarkus issue than a camel one so we should probably let the developers choose.

@ppalaga
Copy link
Contributor Author

ppalaga commented Jan 20, 2020

It looks to me a generic quarkus issue than a camel one so we should probably let the developers choose.

I was about to say that we are letting the 80% of devs to learn the hard way but then tried to find some data about the usage of the individual encodings. Found only https://w3techs.com/technologies/overview/character_encoding that is perhaps a bit Western/US-centric because it is considering just 10M most popular sites, popularity being defined by data from Alexa. But anyway, UTF-8 is most probably dominating nowadays.

For the record, the encodings available by default are:

Charset.defaultCharset(), US-ASCII, ISO-8859-1, UTF-8, UTF-16BE, UTF-16LE, UTF-16

https://github.com/oracle/graal/blob/79d85a3f0a41bb264dd396f6ff335da29525bcd1/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/LocalizationFeature.java#L116-L128

Let's just document on the individual extension pages that the users may need to set quarkus.native.enable-all-charsets = true if that set is not enough for them

@ppalaga ppalaga changed the title Produce NativeImageEnableAllCharsetsBuildItem where appropriate Document the need to set quarkus.native.enable-all-charsets = true in HTTP extensions Jan 21, 2020
@ppalaga ppalaga changed the title Document the need to set quarkus.native.enable-all-charsets = true in HTTP extensions Document the need to set quarkus.native.add-all-charsets = true in HTTP extensions Jan 21, 2020
ppalaga added a commit to ppalaga/camel-quarkus that referenced this issue Jan 21, 2020
ppalaga added a commit to ppalaga/camel-quarkus that referenced this issue Jan 22, 2020
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