Fix regexp in ruby-client #1521
Conversation
| Assert.assertEquals(op.allParams.get(4).pattern, "/^pattern$/im"); | ||
| // pattern_dont_escape_backslash '/^pattern\d{3}$/i' NOTE: the double \ is to escape \ in string but is read as single \ | ||
| Assert.assertEquals(op.allParams.get(5).pattern, "/^pattern\\d{3}$/i"); | ||
| } |
There was a problem hiding this comment.
@meganemura I suggest we keep these tests to ensure the change won't break the previous use cases.
There was a problem hiding this comment.
Please see this comment
#1521 (comment)
meganemura
force-pushed
the
ruby-fix-regexp
branch
from
2851cbe
to
00359b0
Compare
| // pattern_one_slash_end '^pattern$/' | ||
| Assert.assertEquals(op.allParams.get(3).pattern, "/^pattern$/"); | ||
| // pattern_one_slash_near_end '^pattern$/im' | ||
| Assert.assertEquals(op.allParams.get(4).pattern, "/^pattern$/im"); |
There was a problem hiding this comment.
@wing328 @ggershoni
This PR does not have backward compatibility for these 3 patterns in definitions.
But, it seems that those regexp patterns are not correct regular expressions.
I think we do not need to support. WDYT?
There was a problem hiding this comment.
I was trying to ensure that if forward slashes where not surrounding the inputted regex then they would be added to force the inputted text to be a treated as a regex literal... I wanted to prevent someone from putting in malicious code via pattern. The Mustache template doesn't surround the regex with quotes anymore.
I was thinking we could put forward slashes in the Mustache template but then we might have an ending forward slash when we didn't want one i.e. /^pattern$/i/.
There was a problem hiding this comment.
@meganemura I read the code in addRegularExpressionDelimiter and I like it. Thanks heaps for the refactor!
@wing328 I am happy to have those 3 tests removed. @meganemura is right, they are not correct regular expressions.
There was a problem hiding this comment.
@ggershoni Thanks for reviewing 👍
|
@meganemura thanks for the PR, which has been included in the v3.3.4 release: https://twitter.com/oas_generator/status/1068772409795207168 |
* Fix regexp in ruby-client * Remove tests for unknown regexp patterns
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\.master,3.4.x,4.0.x. Default:master.Description of the PR
(details of the change, additional tests that have been done, reference to the issue for tracking, etc)
This fixes #1520.