[PATCH] HttpXsltModule files relative to location root

Maxim Dounin mdounin at mdounin.ru
Mon Jan 2 23:26:03 UTC 2012


On Mon, Jan 02, 2012 at 11:24:04PM +0100, SamB wrote:

> Hi,
>   I've made a (relatively) simple patch for HttpXsltModule, that adds 2 new
> config directives and slightly modifies the xslt parsing process in
> HttpXsltModule:
>   xslt_use_root - enables new xslt file location mode. If enabled
> non-absolute filenames are being located relatively to root location (if
> root defined without variables), and not relatively to nginx work
> directory. To achieve this xslt file parsing has been moved from directive
> processing routine, to merge routine and file location has been extended
> accordingly.
> This makes xslt files deploying easier, especially when deploying beta
> files (in testing) to production site. Before it has been necessary to
> modify file paths to match production site config, including modification
> of xslt include/import directives urls, after enabling it, simple file copy
> + nginx restart is enough.

This isn't taking into account root with variables, as well as 
alias used instead of root.  (And the patch doesn't even provide 
any diagnostics for the case when it's not going to work.)  Apart 
from this, you probably don't want your xslt files to be exposed 
under document root for security reasons.

I believe correct solution for the use case in question would be 
some form of configuration macros to set both document root and 
location of xslt files from the common prefix.  Right now it may 
be easily done with trivial external text processing.

>   xslt_stylesheet_param - this is equivalent of xslt parameter passed to
> xslt_stylesheet, but this works independently and globally. Implemented as
> configuration directive processing, location parameters merging, and
> finally xslt stylesheet parameters merging.
> Again, xslt configuration is a bit simpler because 'common' xslt variables
> don't need to be configured for each xslt_stylesheet, but can be defined
> globally as needed.

This may be intresting, but:

1. One change at a time, please.  There is no way to accept/review 
changes which are mixed with other changes.  And no unrelated 
changes, please.  If you want change to happen, submit it 
explicitly as a separate patch.

2. Please follow coding style.  There is a lot of style violations 
in your patch.

3. You merge params with ones specified at previous levels.  This 
is incorrect approach, it should follow the same model as other 
array-type variables, i.e. it should be inherited from previous 
levels unless there are own params at the level in question.

Maxim Dounin

More information about the nginx-devel mailing list