> On 17 Dec 2020, at 16:22, Joe Orton <jor...@redhat.com> wrote:
>
> On Wed, Dec 16, 2020 at 07:41:59PM +0000, Nick Kew wrote:
>>> On 16 Dec 2020, at 17:47, Yann Ylavic <ylavic....@gmail.com> wrote:
>>>> Wouldn't this stop matching "application/xml" for instance?
>>>>
>>>> Possibly this test instead:
>>>> if (strncmp(ctype, "text/", 5)
>>>> && (!(x = strstr(ctype, "xml"))
>>>> || x == ctype || !strchr("/+", x[-1]))) {
>>>> ?
>>>
>>> I would even remove the "text/" check (why act on "text/plain" for
>>> instance), so maybe:
>>> if (!(x = strstr(ctype, "xml"))
>>> || x == ctype || !strchr("/+", x[-1])
>>> || apr_isalnum(x[3])) {
>>> ?
>>
>> Be liberal in what you accept. You can limit it further in configuration,
>> but you can't override a hardwired check.
>>
>> It certainly needs to operate on text/html for mod_proxy_html, and users
>> might
>> find reasons for running it on other text types as an alternative to an iconv
>> filter like mod_charset(_lite).
>
> I'm not sure if you are agreeing with Yann or not. You wrote the code,
> how do you think we should you resolve PR 64339, NOTABUG & revert
> r1884505 or something else?
Thanks for prompting me to take a proper look at where this thread started.
I started to compose a reply here, bug got bogged down. So I've gone to the
PR instead. I see there's a patch submitted by Giovanni Bechis which looks
more-or-less acceptable, though it does raise further questions.
I've marked the bug NEEDINFO and asked further questions there.
--
Nick Kew
