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

Cannot support APIs that have path parameters that use reserved characters that must not be percent encoded. #2320

Closed
darrelmiller opened this issue Feb 19, 2023 · 6 comments · Fixed by #3201
Assignees
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. WIP
Milestone

Comments

@darrelmiller
Copy link
Member

Currently OpenAPI does not allow Path parameters to use the allowReserved property. This is a problem for APIs that want to use these characters in their APIs. The NightScout v2 API is an example where it uses a comma separated list of values in a path segment.
However, UriTemplates do allow this. https://www.rfc-editor.org/rfc/rfc6570#section-3.2.3

We need to invent a x-AllowReserved as a workaround. OpenAPI didn't allow it because it would make it possible for a parameter to introduce new path segments. That would make operation resolution non-deterministic.

@darrelmiller darrelmiller added the enhancement New feature or request label Feb 19, 2023
@baywet baywet added needs more information generator Issues or improvements relater to generation capabilities. and removed Needs: Triage 🔍 labels Feb 20, 2023
@baywet baywet added this to Kiota Feb 20, 2023
@github-project-automation github-project-automation bot moved this to Todo in Kiota Feb 20, 2023
@baywet baywet added this to the Kiota post-GA milestone Feb 20, 2023
@darrelmiller
Copy link
Member Author

The x-AllowReserved: true property in the OpenAPI would cause the parameter to be rendered as {+myparam} in the path parameter. The + character will prevent reserved characters from being escaped when serializing the parameter value.

@baywet baywet modified the milestones: Kiota post-GA, Kiota 1.1 Feb 24, 2023
@darrelmiller
Copy link
Member Author

After testing with Microsoft Graph we identified that the one place we know of that uses reserved characters in a path parameter, the gateway successfully handles this scenario.
e.g.

https://graph.microsoft.com/v1.0/me/drive/root%3A%2Fattachments%3A/children

Therefore we can move this issue to 1.2.

@sebastienlevert sebastienlevert added the blocked This work can't be done until an external dependent work is done. label Mar 24, 2023
@baywet
Copy link
Member

baywet commented Apr 11, 2023

@darrelmiller, since we don't have "customer demand" for this, and since this is blocked by dependencies, are you ok with moving it to 1.4 for now?

@darrelmiller darrelmiller modified the milestones: Kiota v1.2, Kiota v1.4 Apr 11, 2023
@baywet baywet modified the milestones: Kiota v1.4, Kiota v1.5 Jun 9, 2023
@baywet
Copy link
Member

baywet commented Aug 1, 2023

If I understand this issue correctly we'd need to:

  1. come up with an extension for path parameters https://github.com/microsoft/OpenAPI/tree/main/extensions that documents whether or not reserved expansion is allowed
  2. come up with an OData annotation that documents that aspect
  3. implement the conversion of that annotation to the extension
  4. implement parsing the extension in kiota, and add a + when projecting the path parameter into the template
  5. add an XSLT "fix" for the CSDL to insert the annotation
    Correct?

@darrelmiller
Copy link
Member Author

We don't need this for Graph so we don't need step 2, 3 or 5. Just 1 + 4. I'm fine if we push this to a late milestone due to lack of customer requests.

@baywet
Copy link
Member

baywet commented Aug 25, 2023

Thanks for the precision. It's a small change then I'll go ahead and document the extension and follow up with implementation. Let's leave it in 1.6 for now.

@baywet baywet assigned baywet and unassigned darrelmiller Aug 25, 2023
@baywet baywet removed needs more information blocked This work can't be done until an external dependent work is done. labels Aug 25, 2023
@baywet baywet moved this from Todo to In Progress in Kiota Aug 25, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Kiota Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. WIP
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants