hercules-team / augeas

A configuration editing tool and API
http://augeas.net/
GNU Lesser General Public License v2.1
486 stars 199 forks source link

The NGINX lens reports "types" as a syntax error incorrectly #834

Open johnnybubonic opened 4 months ago

johnnybubonic commented 4 months ago

Place the following as /etc/nginx/conf.d/test.conf:

server {
    listen 127.0.0.1:80;
    server_name poc.localhost;

    types {
            text/plain h;
    }

    location / {
        root /srv/http;
    }
}

Ensure it is parsed by Nginx:

# grep -E '^\s*include\s+' /etc/nginx/nginx.conf
    include       mime.types;
    include /etc/nginx/conf.d/*.conf;

# nginx -t
nginx: the configuration file /etc/nginx/nginx.conf syntax is ok
nginx: configuration file /etc/nginx/nginx.conf test is successful

# echo $?
0

As you can see, the types directive as a block is perfectly valid and a block is expected per the example in Nginx documentation.

Now try to load it.

augtool> load
augtool> print /augeas/files/etc/nginx/conf.d/test.conf/error
/augeas/files/etc/nginx/conf.d/test.conf/error = "parse_failed"
/augeas/files/etc/nginx/conf.d/test.conf/error/pos = "131"
/augeas/files/etc/nginx/conf.d/test.conf/error/line = "12"
/augeas/files/etc/nginx/conf.d/test.conf/error/char = "0"
/augeas/files/etc/nginx/conf.d/test.conf/error/lens = "/usr/share/augeas/lenses/dist/nginx.aug:121.10-.52:"
/augeas/files/etc/nginx/conf.d/test.conf/error/message = "Syntax error"

The obvious reason is that type is not noted as a block directive (neither in block_re nor block_re_all). However, unlike other block_re_all, it only has the form of <directive> <block>, there are no args.

I tried fiddling with the lens a bit but wasn't able to get it to properly pull in the block itself.

tupyy commented 2 months ago

hello, your problem is not there. It's the / in text/plain (actually is the plain). See lens nginx https://github.com/hercules-team/augeas/blob/89d7bc419c44e7f6972c2e06375b2259ee1cacc7/lenses/nginx.aug#L49 It's removing the / to save the mask of the IP and it wants an integer. Replace plain with 24 and it will work.

A quick fix will be to replace the mask with:

in let mask = [ label "mask" . Util.del_str "/" . store Rx.word ]

and you'll obtain something like:

augtool> print etc/nginx/conf.d/
/files/etc/nginx/conf.d
/files/etc/nginx/conf.d/test.conf
/files/etc/nginx/conf.d/test.conf/server
/files/etc/nginx/conf.d/test.conf/server/listen = "127.0.0.1:80"
/files/etc/nginx/conf.d/test.conf/server/server_name = "poc.localhost"
/files/etc/nginx/conf.d/test.conf/server/types
/files/etc/nginx/conf.d/test.conf/server/types/text = "h"
/files/etc/nginx/conf.d/test.conf/server/types/text/mask = "plain"
/files/etc/nginx/conf.d/test.conf/server/location
/files/etc/nginx/conf.d/test.conf/server/location/#uri = "/"
/files/etc/nginx/conf.d/test.conf/server/location/root = "/srv/http"

But, IHO, is not clean and it will break something if users do math op with the mask

or

--- a/lenses/nginx.aug
+++ b/lenses/nginx.aug
@@ -33,10 +33,11 @@ autoload xfm

 (* Variable: word *)
 let word = /[A-Za-z0-9_.:-]+/
+let str = /[A-Za-z]*/

 (* Variable: block_re
      The keywords reserved for block entries *)
-let block_re = "http" | "events" | "server" | "mail" | "stream"
+let block_re = "http" | "events" | "server" | "mail" | "stream" | "types"

 (* All block keywords, including the ones we treat specially *)
 let block_re_all = block_re | "if" | "location" | "geo" | "map"
@@ -47,9 +48,10 @@ let block_re_all = block_re | "if" | "location" | "geo" | "map"
 let simple =
      let kw = word - block_re_all
   in let mask = [ label "mask" . Util.del_str "/" . store Rx.integer ]
+  in let sub_type = [ label "sub_type" . Util.del_str "/" . store str ]
   in let sto = store /[^ \t\n;#]([^";#]|"[^"]*\")*/
   in [ Util.indent .
-       key kw . mask? .
+       key kw . (sub_type | mask)?  .
        (Sep.space . sto)? . Sep.semicolon .
        (Util.eol|Util.comment_eol) ]

@georgehansper WDYT