Regression in 0.7.1 for actix web & tail matches

[cetra3]

Looks like there has been a change to the way url path parameters are generated between 0.7.0 & 0.7.1. When using a tail match, which is normally defined by with a {param:.*} in the url, this now has broken output.

Here's an example config:

cfg.service(web::resource("/example/{filename:.*}").route(web::get().to(get_example)))

This matches anything that has the /example/ prefix, including any extra slashes etc..

Here's a diff of a service:

     "/example/{filename:.*}": {
       "get": {
         "operationId": "get_example",
         "parameters": [
           {
             "in": "path",
-            "name": "filename:.*",
+            "name": "filename",
             "required": true,
             "schema": {
               "type": "string"
             },
             "style": "simple"
           },

I would expect in this case the name still is filename:.*

[tiagolobocastro]

Looks like this was done by #430

[tiagolobocastro]

@cetra3 it this is what's happening then I think current behaviour is wrong because it leads to invalid openapi (easy to check on openapi website editor)
@snaggen any thoughts here?

[snaggen]

Looking at https://swagger.io/specification/ i see that the parameter name should follow the following rules:

REQUIRED. The name of the parameter. Parameter names are case sensitive.

If in is "path", the name field MUST correspond to a template expression occurring within the path field in the Paths Object. See Path Templating for further information.
If in is "header" and the name field is "Accept", "Content-Type" or "Authorization", the parameter definition SHALL be ignored.
For all other cases, the name corresponds to the parameter name used by the in property.

So this must follow Path Tempating rules, however I do not find any information about regex being allowed in the path templates?

But, I wonder if it is not the path that is wrong, and should be stripped to match the name? Isn't the regex a actix-web specific thing? Is /example/{filename:.*} really a valid openapi path template? Possibly the regex should be extracted, and moved on to the schema as a pattern?

pattern (This string SHOULD be a valid regular expression, according to the Ecma-262 Edition 5.1 regular expression dialect)

[tiagolobocastro]

Yeah the problem is that the filename on the path differers, so this yields invalid spec :(

Since there is nothing on the spec about regex being valid or other I think we should keep it as it was before.
I also use regex in the openapi at work, and I've yet to see an issue with the openapi tools.

We can add a option to keep or remove regex? But when the option is set to remove, remove it from the path as well.

What do you think?

[snaggen]

I would have thought, stripping the regex out of the path and adding it as a pattern in the schema would be most correct.

Keeping the regex on the name causes it to be visible to users in documentation as part of the name. This is not a huge problem, just a bit ugly.

[tiagolobocastro]

adding it as a pattern in the schema

What do you mean by this?

Keeping the regex on the name causes it to be visible to users in documentation as part of the name

Yeah fair point, which is why I think it should be configurable. In my case for example I need it because I have tooling that depends on it to generate the correct code. Is this case the user is not a regular user, but a dev tool :D

[snaggen]

In the example diff in OP , there are a schema section. A schema section can contain a pattern, which should be a regex. So, by moving the regex from the path and name, to the schema (in the pattern property) we should get the same semantics as the actix path syntax.

[tiagolobocastro]

You mean like:

type: string
pattern: ".*"

Interestingly on the spec Format it says the following:
"The schema exposes two types of fields: Fixed fields, which have a declared name, and Patterned fields, which declare a regex pattern for the field name.

So it seems a field name can be a regex?

[snaggen]

Well, under the Path section for Patterned Fields, it doesn't mention anything about regex at all.... it seems to just mean the /some/{path} syntax. And also, the Patterned Fields refer to matching the path to use, not to restrict it like actix does, so it is a very different semantics.

     "/example/{filename}": {
       "get": {
         "operationId": "get_example",
         "parameters": [
           {
             "in": "path",
            "name": "filename",
             "required": true,
             "schema": {
               "type": "string",
               "pattern": ".*"
             },
             "style": "simple"
           },

is how I think the actix route

cfg.service(web::resource("/example/{filename:.*}").route(web::get().to(get_example)))

should be interpreted since in actix this means the path /example/{filename} where the filename matches ".**

[tiagolobocastro]

Yeah I'd be happy with that. @cetra3 looks good to you too?

How would we do reversal to generate server side code from that yaml? Only if the pattern is explicitly ".*" do we pass that to the actix server?

[snaggen]

How would we do reversal to generate server side code from that yaml? Only if the pattern is explicitly ".*" do we pass that to the actix server?

Well, since the pattern is regex shouldn't it just be reversed, so that the path would be /example/{$name:$pattern} in any case where the schema have a pattern?

[tiagolobocastro]

Oh ok I thought actix didn't quite support regex, it's been a while since I looked at it.

[cetra3]

So to expand upon the issue a little more, we're using the openapi-generator-cli which does a pretty trivial "search & replace" against the path to build up the correct uri. With 0.7.1 this doesn't work, as filename:.* != filename.

To clarify: in actix-web this syntax is not strictly regex, it's a special way of dictating a path route includes a tail match. That is: the param can include forward slashes.

I.e, /example/{filename:.*} matches on:

/example/file.txt
/example/path/to/file.txt
/example/another/file.txt

Whereas /example/{filename} only matches things like:

/example/file.txt
/example/another_file.txt

So I think this statement might not be entirely correct:

should be interpreted since in actix this means the path /example/{filename} where the filename matches ".**

However: I agree that the example given example looks like it would solve the issue.

[snaggen]

I have now pushed a fix for this regression. The pattern in the template is removed when the paths is serialized. And the pattern is also set as a pattern in the parameter information.
I realize that the patterns might not be fully compatible, but it will probably only be a problem in the more advanced cases. So, perhaps just wait and address that when it becomes a problem.

[cetra3]

Hey looks like this one is still an issue in 0.8.0 should I raise another issue or can we reopen this one.

[tiagolobocastro]

Oh then let's reopen it :(