search

Icinga / icingaweb2-module-director

The Director aims to be your new favourite Icinga config deployment tool. Director is designed for those who want to automate their configuration deployment and those who want to grant their “point & click” users easy access to the configuration.
https://icinga.com/docs/director/latest
GNU General Public License v2.0
412 stars 201 forks source link

iconv problem in containers #2052

Closed lazyfrosch closed 11 months ago

lazyfrosch commented 4 years ago

Expected Behavior

Download should work.

Current Behavior

When downloading an Basket, it fails by showing HTML as text/plain in the browser, listing the exception:

iconv(): Wrong charset, conversion from `UTF-8' to `ISO-8859-11//IGNORE' is not allowed

This code causes it:

https://github.com/Icinga/icingaweb2-module-director/blob/606d71501ef1486a4aac847ccd2a52f8a380736e/application/controllers/BasketController.php#L179-L187

I'm not sure why we are casting to ISO-8859-1 here, but it seems to fail on some platforms.

Steps to Reproduce (for bugs)

For now I use php on Alpine in lazyfrosch/icingaweb2

Your Environment

Thomas-Gelf commented 4 years ago

Seems to be an iconv issue in Alpine Linux. Similar issues:

lazyfrosch commented 4 years ago

The magic of multiple implementations... Do we really need these cast?

Thomas-Gelf commented 4 years ago

ISO-8859-11 is a typo, this should read ISO-8859-1. But this shouldn't really matter and not be related to your issue. As far as I can remember, casting takes place as some browsers have issues with UTF-8. Problem is that RFC 2183 required only US-ASCII, and not all browsers understand encodings as of RFC5987. Please don't ask which versions might be affected, some googling shows that at least Safari used to be.

Thomas-Gelf commented 4 years ago

This is currently the only direct use of iconv, and it might be replaced with another piece of "ignore special characters"-code.

Currently there is at least one more use of iconv with //IGNORE in the pipeline. The communication with the Director Daemon Control Socket uses JSON-RPC. This will be an essential component for many upcoming features, as we'll delegate more and more caching/rendering-logic to the daemon. JSON is strict about UTF8, that's why unsafe messages travel through iconv('UTF-8', 'UTF-8//IGNORE', $message). The new JsonRpcLogWriter does so, as it might forward exceptions with unknown / third party or even partially binary messages.

So while replacing this single iconv() might help in the short run, I do not see not using iconv at all being a viable option.

lazyfrosch commented 4 years ago

Of course not the problem of Director, just want to make sure if we really need the iconv statement here.

Thomas-Gelf commented 4 years ago

@lazyfrosch: did you dig deeper into this issue? As far as I understood, this happens because of PHP binaries compiled in a "rough" way. Is that assumption true? Any idea of how to detect this issue via PHP application code? It doesn't feel good to know that there might be installations with half-broken iconv out there. Director should warn everybody running it in such an environment.

Additional input on this issue would be highly appreciated!

lazyfrosch commented 4 years ago

The problem seems to be that iconv in musl is not implemented to support that conversion, when using GNU iconv it works.

This change helps my problem: https://github.com/lazyfrosch/docker-icingaweb2/commit/353c5d03e898bea0f4bf43e6fb402cb868ca4e1f

I don't need any further investigation.