Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Regular expressions which have forward slash generate invalid ruby code. #5582

Closed
4 of 5 tasks
mkanoor opened this issue Mar 11, 2020 · 2 comments · Fixed by #16474
Closed
4 of 5 tasks

[BUG] Regular expressions which have forward slash generate invalid ruby code. #5582

mkanoor opened this issue Mar 11, 2020 · 2 comments · Fixed by #16474

Comments

@mkanoor
Copy link

mkanoor commented Mar 11, 2020

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?

  • Have you validated the input using an OpenAPI validator ([example]

  • What's the version of OpenAPI Generator used?

  • [X ] Have you search for related issues/PRs?

  • What's the actual output vs expected output?

  • [Optional] Bounty to sponsor the fix (example)

Description

In Regular expression the forward slash only needs to be escaped if its inside the /pattern/ format, if the pattern is a string and fed into Regexp.new it doesn't need to be escaped with a backslash.


This code only checks if the string starts with a leading slash and assumes it's the /pattern/ format. The pattern can be a string and it could include both the leading and trailing slash e.g /root/.
Since Openapi regex pattern supports both the string pattern and the /pattern/ format its ambiguous what the code should generate.
For the pattern '/ax$|/bx$' the ruby code generated is Regexp.new(/ax$|/bx$) which fails with a syntax error
SyntaxError ((irb):20: unknown regexp option - b)
(irb):20: `$)' is not allowed as a global variable name
(irb):20: syntax error, unexpected end-of-input, expecting ')'
Regexp.new(/ax$|/bx$)

Can you please clarify if this a bug or that openapi regexes need to be always in the /pattern/ format.

openapi-generator version

4.2.2 also tried with master

OpenAPI declaration file content or url

https://gist.github.com/mkanoor/802eb64217e04e4e7b5fc12ac4ebf8c5

Command line used for generation

openapi-generator generate -i /tmp/test_regex.yaml -g ruby -o /tmp

Ruby

Steps to reproduce

openapi-generator generate -i /tmp/test_regex.yaml -g ruby -o /tmp

Related issues/PRs

#1392

Suggest a fix

Checking if there is a leading slash and a trailing slash before deciding if this is a /pattern/ format versus a string. That works as long as the regexp is not looking for a leading and trailing slash.

@auto-labeler
Copy link

auto-labeler bot commented Mar 11, 2020

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

mkanoor added a commit to mkanoor/catalog-api that referenced this issue Mar 11, 2020
This is to fix an issue in the openapi generator
When we have regular expressions they come in 2 formats
1. /pattern/
2. "pattern"

In the first one if the pattern contains a forward slash
(aka Solidus from
https://www.ecma-international.org/archive/ecmascript/2013/GA/ga-2013-076.pdf)
it needs to be escaped with a backslash (reverse solidus)

In the second case we dont have to escape the forward slash

In our openapi spec we have always tended to use the second format.

The openapi generator checks the first charachter and if it starts
with a forward slash it assumes it's the first format and doesn't
escape the / contained in the string

https://github.com/OpenAPITools/openapi-generator/blob/de2753dfc7d17a8afcc14fdd705ade3f6cca3cb2/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java#L4825

In our tags we are expecting values like
/approval/workspace=55

Since our regexp starts with a / it bypasses the escaping process in the
openapi generator

To temporarily resolve this issue I have modified our regular expression to have a ^
which bypasses the first charachter to be / in the openapi-generator

I have opened an issue with the openapi-generator repository which might
resolve the issue on the generator
OpenAPITools/openapi-generator#5582
@ivgiuliani
Copy link
Contributor

Just bumped into this issue. I think this is the same problem as #1517 but for Ruby, however because Python/Ruby both use perl-flavored regexes, I imagine the fix is likely going to be very similar to what was done in #1539 (I have a test PR that I will try to clean up now that backports the python fix to the ruby code generator and it seems to work).

ivgiuliani added a commit to ivgiuliani/openapi-generator that referenced this issue Sep 1, 2023
Ruby is not correctly escaping pattern sequences containing forward
slashes in their definition. This commit adds tests that verify the
correct behaviour of the code generator.

See issue OpenAPITools#5582.
ivgiuliani added a commit to ivgiuliani/openapi-generator that referenced this issue Sep 1, 2023
Ruby regexs are always generated as match patterns enclosed in slash
characters (i.e. using the `/pattern/` syntax). Regular expressions
defined in the OpenAPI declaration via the `pattern` attribute follow
[ECMA 262](https://262.ecma-international.org/5.1/#sec-15.10.1) which
means they already include the correct escaping of forward slashes as
far as Ruby is concerned.

The current Ruby codegen is incorrectly escaping all forward slashes,
which ultimately causes the generated code to include additional
incorrect escape sequences which cause the generated file to have an
invalid syntax.

This commit ports the same fix introduced in OpenAPITools#1539 for the Python
codegen, as both Ruby and Python use perl-flavored regular expressions
so they behave in the same way when it comes to escaping forward
slashes.

Fixes OpenAPITools#5582.
wing328 pushed a commit that referenced this issue Sep 5, 2023
* [Ruby] Test correct escaping of pattern sequences

Ruby is not correctly escaping pattern sequences containing forward
slashes in their definition. This commit adds tests that verify the
correct behaviour of the code generator.

See issue #5582.

* [Ruby] Correctly escape patterns containing forward slashes

Ruby regexs are always generated as match patterns enclosed in slash
characters (i.e. using the `/pattern/` syntax). Regular expressions
defined in the OpenAPI declaration via the `pattern` attribute follow
[ECMA 262](https://262.ecma-international.org/5.1/#sec-15.10.1) which
means they already include the correct escaping of forward slashes as
far as Ruby is concerned.

The current Ruby codegen is incorrectly escaping all forward slashes,
which ultimately causes the generated code to include additional
incorrect escape sequences which cause the generated file to have an
invalid syntax.

This commit ports the same fix introduced in #1539 for the Python
codegen, as both Ruby and Python use perl-flavored regular expressions
so they behave in the same way when it comes to escaping forward
slashes.

Fixes #5582.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants