[Slim] Encode path to support non-latin characters#1687
[Slim] Encode path to support non-latin characters#1687wing328 merged 4 commits intoOpenAPITools:masterfrom
Conversation
There was a problem hiding this comment.
@ybelenko why do we need to replace /$ with empty string?
There was a problem hiding this comment.
I just copied it from:
https://github.com/OpenAPITools/openapi-generator/blob/e15710815c149810ba9a46176a050f8ca04eefd9/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java#L199
I guess that it removes trailing slash.
There was a problem hiding this comment.
@wing328 Maybe I can change basePathWithoutHost somewhere else? In some post process functions in Slim generator? Or maybe we should add encodedBasepathWithoutHost property? I think that this problem with non-latin characters exists in other generators as well.
There was a problem hiding this comment.
I think that this problem with non-latin characters exists in other generators as well.
I think so but I didn't have time to test and confirm.
There was a problem hiding this comment.
My take is to start with your fix for Slim generator and later we port this fix to the default codegen or other generators if users report similar issues.
There was a problem hiding this comment.
Yeah, I agree, it's not so important now.
Important part is basePathWithoutHost value. Whether we should override it or put it in separated prop. Do you know how to override it inside PhpSlimServerCodegen codegen class?
What contextPath property stands for?
Both variables basePathWithoutHost and path are already urlEncoded in codegen itself. Builtin html encoding in mustache is redundant. We can use these raw codegen values with no fear.
* master: (26 commits) Delete unused method (OpenAPITools#1744) Use JsonNullable wrapper on nullable/x-nullable fields (OpenAPITools#1762) Add an option to use reflection in equals, hashCode (Java client) (OpenAPITools#1767) [Slim] Encode path to support non-latin characters (OpenAPITools#1687) [elm] Add support for sending headers (OpenAPITools#1704) Add test case for InlineModelResolver: inline array response (OpenAPITools#1778) fix group parameter logic (OpenAPITools#1779) Add test case for InlineModelResolver: inline array request body (OpenAPITools#1777) Add test case for InlineModelResolver: inline array schema (OpenAPITools#1772) Fix type inference error (OpenAPITools#1773) skip default value for contaier in spring (OpenAPITools#1725) [Slim] Add PHP CodeSniffer config template (OpenAPITools#1764) Use CompareNetObject for object comparison in C# client (refactor) (OpenAPITools#1765) Add test case for InlineModelResolver (OpenAPITools#1771) Add online gen tests (OpenAPITools#1759) Resolve inline models before preprocess (OpenAPITools#1761) better handling of allOf (composition) (OpenAPITools#1757) Fix UUID support (OpenAPITools#1746) Use appInfo.version for podspec (OpenAPITools#1760) [Swift 4] Add `createURLRequest` method (OpenAPITools#1727) ...
|
@ybelenko thanks for the contribution, which has been included in the 4.0.0-beta release: https://twitter.com/oas_generator/status/1079727020374806529. Happy New Year and looking forward to more collaboration and contributions in 2019! |
* [Slim] Add encodePath method * [Slim] Add tests for encodePath method * [Slim] Use unescaped path in router Both variables basePathWithoutHost and path are already urlEncoded in codegen itself. Builtin html encoding in mustache is redundant. We can use these raw codegen values with no fear. * [Slim] Refresh samples
2 participants
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,. Default:3.4.x,4.0.xmaster.Description of the PR
Closes: #766
This PR should be reviewed carefully, because I didn't find a way to fix it without DefaultGenerator modification.
As it turns out
{{basePathWithoutHost}}construction doesn't solve issue when path contains non-latin characters. For instance, Slim router cannot find related API controller when path looks like/статьи/авторы, because escaped mustache variable output is/статьи/авторыwithout any difference, while HTTP clients and web browsers encodes non-latin characters by default. When I use this link in Chrome I see in server logs that url has been automatically encoded into/%D1%81%D1%82%D0%B0%D1%82%D1%8C%D0%B8/%D0%B0%D0%B2%D1%82%D0%BE%D1%80%D1%8B. The same behaviour with Insomnia app. With new methodencodePathSlim router works as expected.I've also added few tests to check
encodePathmethod output ofPhpSlimServerCodegen.cc @jebentier @dkarlovi @mandrean @jfastnacht @ackintosh @renepardon