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

Parameters declared by $ref are skipped #803

Closed
tom-inetum-realdolmen opened this issue Jan 12, 2023 · 4 comments · Fixed by #962
Closed

Parameters declared by $ref are skipped #803

tom-inetum-realdolmen opened this issue Jan 12, 2023 · 4 comments · Fixed by #962

Comments

@tom-inetum-realdolmen
Copy link

Describe the bug
Parameters that are declared as a $ref are skipped

To Reproduce
file openapi.yaml has this portion:

openapi: 3.0.3
info:
  title: Parameter ReferenceObject
  description: test
  version: '1'
  termsOfService: https://www.google.be
  contact:
    email: [email protected]
security:
  - basicAuth: []
servers:
  - url: http://localhost
    description: local host
paths:
  /subject:
    get:
      summary: Test
      operationId: doTest
      parameters:
        - in: query
          name: firstParam
          schema: 
           type: string
        - $ref: "#/components/parameters/FromDateTime"
        - in: query
          name: secondParam
          schema:
            $ref: "/components/schemas/SecondParam"
      responses:
       ... (abbreviated for clarity)
components:
  schemas:
    SecondParam:
      type: string

  parameters:
    FromDateTime:
      in: query
      name: fromDateTime
      required: false
      schema: 
        type: string

Actual behavior
parameter fromDateTime is present in req.openapi.schema.parameters but is not translated/parsed

Expected behavior
parameter fromDateTime is fully present in req.openapi.schema.parameters

Examples and context
see issue bug-hunters/oas3-tools#59

@shashanksapre
Copy link

Same. I am having to define them inline for each endpoint. This goes for both path and query parameters. :(

@cdimascio
Copy link
Owner

@tom-inetum-realdolmen the provided spec appears to have errors
for instance,
$ref: "/components/schemas/SecondParam" is missing the #
and should be
$ref: "/components/schemas/SecondParam"

can you retry with the fix

@duncanbeevers
Copy link
Contributor

duncanbeevers commented Sep 2, 2024

I'm reproducing this error locally, and have traced it to the behavior of ajv when validating + coercing types at https://github.com/cdimascio/express-openapi-validator/blob/master/src/middlewares/openapi.request.validator.ts#L188

Old, failing test case (pre-#962) I have a reproduce case, but since I think it needs fixing upstream I'm holding off on a PR. However, I'll see if I can stick a pin in it today.
diff --git a/test/operation.handler.spec.ts b/test/operation.handler.spec.ts
index b7e60fe..faf3f5a 100644
--- a/test/operation.handler.spec.ts
+++ b/test/operation.handler.spec.ts
@@ -120,4 +120,13 @@ describe('custom operation handler', () => {
       });
   });
 
+  it('should coerce path parameters', async () => {
+    return request(app)
+    .get(`${basePath}/users/123/info`)
+    .expect(200)
+    .then((r) => {
+      expect(r.text).to.be.equal('{"id":123}');
+    });
+  });
+
 });
diff --git a/test/resources/eov-operations.modulepath.yaml b/test/resources/eov-operations.modulepath.yaml
index 1eb397d..2c498ba 100644
--- a/test/resources/eov-operations.modulepath.yaml
+++ b/test/resources/eov-operations.modulepath.yaml
@@ -185,8 +185,39 @@ paths:
                 properties:
                   success:
                     type: boolean
+  /users/{userID}/info:
+    get:
+      description: Get info about a User
+      summary: Get info about a User
+      operationId: user.info
+      security: []
+      parameters: 
+        - $ref: '#/components/parameters/userID'
+        # - name: userID
+        #   in: path
+        #   required: true
+        #   schema: 
+        #     type: number
+      responses:
+        '200':
+          description: Returns info about a User.
+          content:
+            'application/json':
+              schema:
+                type: object
+                properties:
+                  id:
+                    $ref: '#/components/schemas/UserID'
 
 components:
+  parameters:
+    userID:
+      name: userID
+      in: path
+      required: true
+      schema: 
+        $ref: '#/components/schemas/UserID'
+
   schemas:
     Pet:
       required:
@@ -210,6 +241,9 @@ components:
         - dog
         - cat
 
+    UserID:
+      type: number
+
     Error:
       required:
         - code

Opened a PR to fix this; see the PR + commit message for details about what's happening.
#962

duncanbeevers added a commit to duncanbeevers/express-openapi-validator that referenced this issue Sep 4, 2024
The OpenAPI spec loader has a `discoverRoutes` method which explores an OpenAPI document
and gathers information about the paths and parameters used.
The list of discovered path parameters is used to install parameter-specific middleware in `src/openapi.validator.ts#installPathParams`
Path parameters declared with `$ref` were not detected in the `discoverRoutes` implementation, leading to the un-coerced values being used.
By dereferencing each path parameter when building this list, we should see the same behavior for referenced path parameters and for inline path parameters.

Closes cdimascio#803
@duncanbeevers
Copy link
Contributor

@shashanksapre I didn't see anything while fixing this bug that indicated it should affect query parameters.
When I extended the test case here with a numeric query parameter it came through in req.query coerced to the correct type, as expected.

Please ping me if you're still seeing this issue affecting query parameters, after applying this fix.

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 a pull request may close this issue.

4 participants