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

fix(o2k): limit path-param names to 32 chars #153

Merged
merged 2 commits into from
Feb 15, 2024
Merged

fix(o2k): limit path-param names to 32 chars #153

merged 2 commits into from
Feb 15, 2024

Conversation

Tieske
Copy link
Member

@Tieske Tieske commented Feb 15, 2024

PCRE engines have a limit of 32 characters.

the path-parameter-names need to be predictable, because they might reuse them in other places (request-transformer template for example). So truncation or truncation and replacing the last x-chars with a hash for uniqueness, will not work, since the names become unpredictable.

Then I think the only alternative is to throw an error in OAS2kong, and have the user preprocess them to shorter names, such that they are well informed about the new names in use. More work, but no surprises.

the path-parameter-names need to be predictable, because they might reuse them in other
places (request-transformer template for example). So truncation or truncation and replacing
the last x-chars with a hash for uniqueness, will not work, since the names become unpredictable.

Then I think the only alternative is to throw an error in OAS2kong, and have the user preprocess
them to shorter names, such that they are well informed about the new names in use. More work,
but no surprises.
Copy link
Member

@mheap mheap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some tests

@Tieske
Copy link
Member Author

Tieske commented Feb 15, 2024

working on it

@Tieske Tieske merged commit 7e137bb into main Feb 15, 2024
3 checks passed
@Tieske Tieske deleted the limit-capture branch February 15, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants