[ms-rest] Using \W instead of \w for regex match to include some characters #2175
Conversation
| @@ -163,7 +163,7 @@ class WebResource { | |||
| } | |||
| let baseUrl = options.baseUrl; | |||
| let url = baseUrl + (baseUrl.endsWith('/') ? '' : '/') + (options.pathTemplate.startsWith('/') ? options.pathTemplate.slice(1) : options.pathTemplate); | |||
| let segments = url.match(/({\w*\s*\w*})/ig); | |||
| let segments = url.match(/({\W*\S*\W*})/ig); | |||
There was a problem hiding this comment.
I'm not sure I understand the fix. With this change, a text like {foo bar} that used to be a match for the old pattern, will no longer be a match. Is that intentional, why?
In all, this change can be summarized as so :
- change looking for any word character to any non word character
- change looking for a space to any non-whitespace character
I don't understand this change, feel free to drop by and talk to me 😃
There was a problem hiding this comment.
Yeah, not sure I understand it either. Is "-" the only special char we want to allow or do we allow others? if so, should we just have the expression accept "-" between words?
I'm not sure about the original regex either though, do we allow spaces in between words for the property inside {}?
There was a problem hiding this comment.
@balajikris @veronicagg Yes those are valid questions. I am also not sure why the initial regex was allowing white space / tabs between {}.
Secondly, after looking around I saw that technically swagger path templating & rfc6570 does not say that - or space is not allowed. Moreover, sway and it's regex matcher intentionally decided not to support -.
The question remains for us is, path /storage/{storage-account-name} seems to be valid in keyvault/2016-10-01/swagger/keyvault.json with parameter defined as "name": "storage-account-name". The swagger validation tool does not complain about semantic validation but
- From linter perspective, do we have any reason for this to be camelCased? For example similar to Rule M3016
- From model validation perspective how to approach this?
Let me know your thoughts!!
fixes #2174