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_dieter...@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"). >