Parsing context does not change when finding block directives with wrong number of arguments

[dareste]

Describe the bug

When the parser finds a block with a wrong number of arguments, the error is collected but the context does not change. This means we continue to ingest tokens and analyze statements under a wrong context.

Also, if the parent context is NGX_MAIN_CONF, the next closing bracket token will be interpreted as the end of file, and parsing will stop.

To reproduce

Analyze the following configuration:

user nginx;
worker_processes auto;
events  {
    worker_connections  1024;
}
http 123 {
   [http directives]
}
[more directives]
...

this will produce:

• One error for the http directive (wrong number of parameters).
• Errors for all the [http directives] (wrong context).
• Premature end of parsing before [more directives] are consumed.

Expected behavior

A block directive with a wrong number of arguments (and probably other types of errors as well) should trigger a change of context in the analyze function.

[ornj]

Here is the config that I tested:

user  nginx;
worker_processes  auto;
error_log  /var/log/nginx/error.log notice;
pid        /var/run/nginx.pid;

events {
    worker_connections  1024;
}

http 123 {
    server {
    listen       80 default_server;
    server_name  localhost;

    location / {
        root   /usr/share/nginx/html;
        index  index.html index.htm;
    }
}

stream {
    server {
        listen 127.0.0.1:53 udp reuseport;
        proxy_timeout 20s;
        proxy_pass dns;
    }
}

When I parse this with StopParsingOnError: true then I get this error returned from crossplane.Parse:

invalid number of arguments in "http" directive in nginx.conf:11

nginx -t returns a similar error.

nginx: [emerg] invalid number of arguments in "http" directive in /etc/nginx/nginx.conf:11

If StopParsingOnError == false then Crossplane continues to parse the http directive's block and complains that "{directive} does not belong here" which I think is the observed behavior you are calling out.

To make sure I understand I'll repeat what I think your expected behavior is. Your suggestion is when StopParsingOnError == false crossplane.Parse should attempt to skip over the invalid http directive's block and resume reporting errors with the next directive in the current context?

[dareste]

If StopParsingOnError == false then Crossplane continues to parse the http directive's block and complains that "{directive} does not belong here" which I think is the observed behavior you are calling out.

Correct. These are wrongly called out as errors due to the fact that they are being evaluated under NGX_MAIN_CONF. Expected behavior should be that these directives were evaluated under NGX_HTTP_MAIN_CONF.

Another problem is that this is leading to an early termination of parsing due to the fact that the http closing bracket is misinterpreted as the end of the main context. Compare the outputs that crossplane produces when StopParsingOnErroris false, with and without the wrong parameter in http (btw, there's a missing closing bracket before stream):

error-in-http-args.json no-error.json

As you'll see, config.parsed is missing all the stream block in the first case.