Malformed http request URL

bauti-defi commented 1 year ago

Issue workflow progress

Progress of the issue based on the Contributor Workflow

[ ] 1. The issue provides a reproduction available on Github, Stackblitz or CodeSandbox

Make sure to fork this template and run yarn generate in the terminal.

Please make sure Mesh package versions under package.json matches yours.

[ ] 2. A failing test has been provided
[ ] 3. A local solution has been provided
[ ] 4. A pull request is pending review

Describe the bug

The graphql-mesh server is making requests with malformed URLs.

To Reproduce Steps to reproduce the behavior:

I cannot provide everything required to reproduce out of the box. Here is the general idea.

I have done a lot of debugging but reached a dead end that may lead into the graphql-mesh codebase.

My mesh server has a openapi-3 source handler that consumes an internal API.

sources:
  - name: Api
    handler:
      openapi:
        endpoint: http://localhost:5050/api/v1/
        source: ./schemas/openapi3-api.json

It constructs the schema based on a openapi3-api.json spec. Here is a yaml formatted snippet from that spec file. This is a single http GET route.

 /nfts/{nftAddress}/{tokenId}:
    get:
      tags:
        - NFTs
      description: Returns NFT
      parameters:
        - name: nftAddress
          in: path
          description: The NFT address
          required: true
          schema:
            $ref: '#/components/schemas/Address'
        - name: tokenId
          in: path
          description: The token ID
          required: true
          schema:
            type: integer
        - name: chain
          in: query
          description: The blockchain name
          required: false
          schema:
            $ref: '#/components/schemas/Chain'
      responses:
        200:
          description: successful operation
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/NonFungibleToken'

It builds, runs and queries! But i have found an edge case which causes the above route to not work as expected.

_

example:

Making this graphql request to the mesh server:

query MyQuery {
  nfts_by_nftAddress_by_tokenId(nftAddress: "0x9e9c14f909f441bbf0e7a688f7df76e133ee8873", 
tokenId:1, 
chain:_1){
    nftAddress
  }
}

Produces the following http request from the mesh server to the api:

GET /api/v1/nfts/0x9e9c14f909f441bbf0e7a688f7df76e133ee8873/1?chain=1 HTTP/1.1 200 11 - undici

This is perfect! No problem here.

Expected behavior

BUT, the following graphql request is the one in question:

query MyQuery {
  nfts_by_nftAddress_by_tokenId(nftAddress: "0x9e9c14f909f441bbf0e7a688f7df76e133ee8873", 
tokenId:0, # THIS IS NOW == 0
chain:_1
){
    nftAddress
  }
}

Notice, the only difference between the former and the latter is the tokenId parameter value.

This graphql request produces the following http request from the mesh server to the api:

GET /api/v1/nfts/0x9e9c14f909f441bbf0e7a688f7df76e133ee8873/[THERE SHOULD BE A 0 HERE]?chain=1 HTTP/1.1 200 11 - undici

This http request will always fail because it does not follow the format specified by the openapi-3 spec file.

Furthermore, any requests with tokenId=0 leads to the same url malformation.

Environment:

OS: macOS 13.1 (22C65)

@graphql-mesh/...:

"@graphql-mesh/cli": "0.82.24",
"@graphql-mesh/graphql": "0.34.7",
"@graphql-mesh/openapi": "0.35.18",
"@graphql-mesh/transform-filter-schema": "0.15.17",
"@graphql-tools/delegate": "9.0.28"

NodeJS: v18.12.1

Additional context

I may post this in the @graphql-mesh/openapi repo.

bauti-defi commented 1 year ago

Hey guys, after further debugging i was always to find and patch the bug. The problem was in the @graphql-mesh/string-interpolator package.

The Fix

Today I used patch-package to patch @graphql-mesh/string-interpolation@0.4.2 for the project I'm working on.

Here is the diff that solved my problem:

diff --git a/node_modules/@graphql-mesh/string-interpolation/cjs/interpolator.js b/node_modules/@graphql-mesh/string-interpolation/cjs/interpolator.js
index b1e0487..4fcdb74 100644
--- a/node_modules/@graphql-mesh/string-interpolation/cjs/interpolator.js
+++ b/node_modules/@graphql-mesh/string-interpolation/cjs/interpolator.js
@@ -99,7 +99,7 @@ class Interpolator {
     }
     applyRule(str, rule, data = {}) {
         const dataToReplace = this.applyData(rule.key, data);
-        if (dataToReplace) {
+        if (typeof dataToReplace !== 'undefined') {
             return str.replace(rule.replace, this.applyModifiers(rule.modifiers, dataToReplace, data));
         }
         else if (rule.alternativeText) {

This issue body was partially generated by patch-package.

Explanation

In my example dataToReplace had an int value of 0. Which is considered falsey in javascript. This causes the if(dataToReplace) conditional to return false and not interpolate the string correctly.

My solution was to check if the values typeof is equal to undefined. Let me know if this solution is good enough. If so, i'd be happy to open up a PR to patch it.

Cheers.

ardatan commented 1 year ago

Thanks for opening the issue! We'd love to accept a PR :)

ardatan / graphql-mesh