You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Nick Kew <ni...@webthing.com> on 2014/02/05 14:57:41 UTC

Re: Revisiting: xml2enc, mod_proxy_html and content compression

On 21 Jan 2014, at 07:14, Ewald Dieterich wrote:

> On 12/17/2013 12:47 PM, Nick Kew wrote:
>> 
>> On 17 Dec 2013, at 10:32, Thomas Eckert wrote:
>> 
>>> I've been over this with Nick before: mod_proxy_html uses mod_xml2enc to do the detection magic but mod_xml2enc fails to detect compressed content correctly. Hence a simple "ProxyHTMLEnable" fails when content compression is in place.
>> 
>> Aha!  Revisiting that, I see I still have an uncommitted patch to make
>> content types to process configurable.  I think that was an issue you
>> originally raised?  But compression is another issue.
> 
> I don't think you committed the patch to make content types configurable. Would you mind to share that patch? I have problems with a SharePoint 2013 server that sends a response with a multipart/related content type and I need to exclude that content type from processing:

OK, I've been meaning to commit for far too long.
Very sorry to have left you in limbo!

The hesitation is because I've been wanting to review the
patch before committing, and round tuits are in woefully
short supply.  So I'm attaching it here.  I'll take any feedback
from you or other users as a substitute for my own review,
and commit if it works for you without glitches.

-- 

Re: Revisiting: xml2enc, mod_proxy_html and content compression

Posted by Ewald Dieterich <ew...@t-online.de>.
On 02/11/2014 06:03 PM, Nick Kew wrote:
>
> On 6 Feb 2014, at 09:40, Ewald Dieterich wrote:
>> My wishlist:
>>
>> * Make the configuration option as powerful as the compiled in fallback so that you can configure eg. "contains xml". But how would you do that? Support regular expressions?
>
> Nice thought.  Perhaps the expression parser would be the ideal solution?

A good idea even if it could be a challenge to configure because the 
expression parser covers so much.

>> * Provide a configuration option to blacklist content types so that you can use the defaults that are compiled in but exclude specific types from processing (this is how I work around the Sharepoint problem, I simply exclude content type "multipart/related").
>
> Perhaps combined with the expression parser as a 'magic' clause that
> expands to the default?

Yes, that would be very convenient.

Re: Revisiting: xml2enc, mod_proxy_html and content compression

Posted by Nick Kew <ni...@webthing.com>.
On 6 Feb 2014, at 09:40, Ewald Dieterich wrote:

> Thanks for the patch!
> 
> On 02/05/2014 02:57 PM, Nick Kew wrote:
>> 
>> The hesitation is because I've been wanting to review the
>> patch before committing, and round tuits are in woefully
>> short supply.  So I'm attaching it here.  I'll take any feedback
>> from you or other users as a substitute for my own review,
>> and commit if it works for you without glitches.
> 
> Minor glitch: the patch doesn't compile because it uses the unknown variable cfg in xml2enc_ffunc().

Damn.  That may mean it's a "wrong" version of the patch.  At least incomplete.
Thanks for picking that up.

> Otherwise it works as advertised.

That's good to hear.

> My wishlist:
> 
> * Make the configuration option as powerful as the compiled in fallback so that you can configure eg. "contains xml". But how would you do that? Support regular expressions?

Nice thought.  Perhaps the expression parser would be the ideal solution?

> * Provide a configuration option to blacklist content types so that you can use the defaults that are compiled in but exclude specific types from processing (this is how I work around the Sharepoint problem, I simply exclude content type "multipart/related").

Perhaps combined with the expression parser as a 'magic' clause that
expands to the default?

${markup-types}

[Thomas Eckert]

> Doesn't
> 
> +    else { 
> +        /* default - only act if starts-with "text/" or contains "xml" */ 
> +        wanted = !strncmp(ctype, "text/", 5) || strstr(ctype, "xml"); 
> +    }
> 
> suffer from the same problem as the original code ?

That *is* the old behaviour, and keeps back-compatibility for the majority
of users who are fine with that.  If the default doesn't work for you then just
set something else!

What do you think of using the expression parser to determine whether to run?

-- 
Nick Kew



Re: Revisiting: xml2enc, mod_proxy_html and content compression

Posted by Thomas Eckert <th...@gmail.com>.
Doesn't

+    else {
+        /* default - only act if starts-with "text/" or contains "xml" */
+        wanted = !strncmp(ctype, "text/", 5) || strstr(ctype, "xml");
+    }

suffer from the same problem as the original code ? So if the user did not
give any "xml2Types" the default behaviour will hit the same problem as
with http://www.mail-archive.com/dev@httpd.apache.org/msg57029.html (the
mentioned earlier discussion regarding ctypes). IMO, this is especially
important seeing how using the new code requires configuration entries to
be added which a lot of admins will either not be aware of or simply not be
doing. In regards to the cited patch section above, I suggest something
along the lines of my patch suggestion in the first message of the
mentioned thread, e.g. include
  !strncmp(ctype, "application", 11)

Otherweise it's fine I think. Will test it sometime this week.



On Thu, Feb 6, 2014 at 10:40 AM, Ewald Dieterich <
ewald_dieterich@t-online.de> wrote:

> Thanks for the patch!
>
>
> On 02/05/2014 02:57 PM, Nick Kew wrote:
>
>>
>> The hesitation is because I've been wanting to review the
>> patch before committing, and round tuits are in woefully
>> short supply.  So I'm attaching it here.  I'll take any feedback
>> from you or other users as a substitute for my own review,
>> and commit if it works for you without glitches.
>>
>
> Minor glitch: the patch doesn't compile because it uses the unknown
> variable cfg in xml2enc_ffunc(). Otherwise it works as advertised.
>
> My wishlist:
>
> * Make the configuration option as powerful as the compiled in fallback so
> that you can configure eg. "contains xml". But how would you do that?
> Support regular expressions?
>
> * Provide a configuration option to blacklist content types so that you
> can use the defaults that are compiled in but exclude specific types from
> processing (this is how I work around the Sharepoint problem, I simply
> exclude content type "multipart/related").
>

Re: Revisiting: xml2enc, mod_proxy_html and content compression

Posted by Ewald Dieterich <ew...@t-online.de>.
Thanks for the patch!

On 02/05/2014 02:57 PM, Nick Kew wrote:
>
> The hesitation is because I've been wanting to review the
> patch before committing, and round tuits are in woefully
> short supply.  So I'm attaching it here.  I'll take any feedback
> from you or other users as a substitute for my own review,
> and commit if it works for you without glitches.

Minor glitch: the patch doesn't compile because it uses the unknown 
variable cfg in xml2enc_ffunc(). Otherwise it works as advertised.

My wishlist:

* Make the configuration option as powerful as the compiled in fallback 
so that you can configure eg. "contains xml". But how would you do that? 
Support regular expressions?

* Provide a configuration option to blacklist content types so that you 
can use the defaults that are compiled in but exclude specific types 
from processing (this is how I work around the Sharepoint problem, I 
simply exclude content type "multipart/related").