You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Thomas Eckert <th...@gmail.com> on 2013/05/29 15:28:33 UTC

Bug with mod_xml2enc and docx files

Downloading a .docx file through a HTML rewriting reverse-proxy suddenly
increased file size by 5 kB. So looking at

  modules/filters/mod_xml2enc :: static apr_status_t
xml2enc_ffunc(ap_filter_t* f, apr_bucket_brigade* bb)

I saw the following

    /* only act if starts-with "text/" or contains "xml" */
    if (strncmp(ctype, "text/", 5) && !strstr(ctype, "xml"))  {
        ap_remove_output_filter(f);
        return ap_pass_brigade(f->next, bb) ;
    }

    (...)

What's this supposed to do with .docx files ? They are (usually) registered
as

  application/vnd.openxmlformats-officedocument.wordprocessingml.document
docx

which I would think not suitable for HTML rewriting. Hence I suggest

diff --git a/modules/filters/mod_xml2enc.c b/modules/filters/mod_xml2enc.c
index 13608ed..0ad35aa 100644
--- a/modules/filters/mod_xml2enc.c
+++ b/modules/filters/mod_xml2enc.c
@@ -336,6 +336,15 @@ static apr_status_t xml2enc_ffunc(ap_filter_t* f,
apr_bucket_brigade* bb)
         return ap_pass_brigade(f->next, bb) ;
     }

+    /* 'docx-patch': do not touch application ctypes, even if they contain
"xml" */
+    if (!strncmp(ctype, "application", 11)) {
+      ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, f->r, "application ctype
detected, removing filter");
+      ap_remove_output_filter(f);
+      return ap_pass_brigade(f->next, bb);
+    }
+
     if (ctx->bbsave == NULL) {
         ctx->bbsave = apr_brigade_create(f->r->pool,
                                          f->r->connection->bucket_alloc);

This of course goes for all such x-typed document formats, since all of
their corresponding mime types start with "application".

Cheers,
  Thomas

Re: Bug with mod_xml2enc and docx files

Posted by Thomas Eckert <th...@gmail.com>.
Would you please let me know once you have completed/commited this ? I much
rather copy your fix locally and apply it until the next upgrade where it
would be contained anyway, instead of live with another custom patch.

On Tue, Jun 4, 2013 at 1:13 AM, Nick Kew <ni...@webthing.com> wrote:

>
> On 3 Jun 2013, at 14:31, Thomas Eckert wrote:
>
> > <mod_xml2enc_ctype.patch>
>
> Thanks.
>
> Looks a lot like the patch I hacked up but have yet to test or commit :)
>
> --
> Nick Kew
>

Re: Bug with mod_xml2enc and docx files

Posted by Nick Kew <ni...@webthing.com>.
On 3 Jun 2013, at 14:31, Thomas Eckert wrote:

> <mod_xml2enc_ctype.patch>

Thanks.

Looks a lot like the patch I hacked up but have yet to test or commit :)

-- 
Nick Kew

Re: Bug with mod_xml2enc and docx files

Posted by Thomas Eckert <th...@gmail.com>.
I attached what I have in mind on how to solve this. It's basically what
you suggested.

There's one thing I didn't know how to address and that is mod_proxy_html's
ProxyHTMLExtended directive. It does suggest not to be turned on by default
(
http://httpd.apache.org/docs/current/mod/mod_proxy_html.html#proxyhtmlextended)
so I wouldn't put it into mod_xml2enc config creation. But then
mod_proxy_html would have to cause mod_xml2enc to include CSS/JS as content
type or we risk the two modules behaving inconsistently for non UTF-8
encoded documents.

Personally, I wouldn't want to rely on the user to remember configuring
ProxyHTMLExtended should always be paired with xml2AcceptContentType
text/css and xml2AcceptContentType application/javascript. That's bound to
cause problems.

I don't know about the caveats of one module telling another module how to
configure itself.

Cheers,
  Thomas

On Wed, May 29, 2013 at 8:20 PM, Nick Kew <ni...@webthing.com> wrote:

>
> On 29 May 2013, at 14:28, Thomas Eckert wrote:
>
> > I saw the following
> >
> >     /* only act if starts-with "text/" or contains "xml" */
> >     if (strncmp(ctype, "text/", 5) && !strstr(ctype, "xml"))  {
> >         ap_remove_output_filter(f);
> >         return ap_pass_brigade(f->next, bb) ;
> >     }
>
> You're right, that's a bug.
>
> I don't think your patch really works either: any hardwired rules are
> going to be susceptible to similar issues.  I guess what it needs is to
> be user-configurable, perhaps with HTML and XHTML as defaults.
>
> --
> Nick Kew
>

Re: Bug with mod_xml2enc and docx files

Posted by Nick Kew <ni...@webthing.com>.
On 29 May 2013, at 14:28, Thomas Eckert wrote:

> I saw the following
> 
>     /* only act if starts-with "text/" or contains "xml" */
>     if (strncmp(ctype, "text/", 5) && !strstr(ctype, "xml"))  {
>         ap_remove_output_filter(f);
>         return ap_pass_brigade(f->next, bb) ;
>     }

You're right, that's a bug.

I don't think your patch really works either: any hardwired rules are
going to be susceptible to similar issues.  I guess what it needs is to
be user-configurable, perhaps with HTML and XHTML as defaults.

-- 
Nick Kew