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

Disallow new lines in paths when checking with isValidPath #6055

Merged
merged 4 commits into from
Apr 15, 2024
Merged

Conversation

enisdenjo
Copy link
Collaborator

@enisdenjo enisdenjo commented Apr 11, 2024

A string may sometimes look like a path but is not (like an SDL of a simple GraphQL schema). To make sure we don't yield false-positives in such cases, we disallow new lines in paths (even though most Unix systems support new lines in file names).

An example of a false-positive:

schema @transport(subgraph: "API", kind: "rest", location: "http://0.0.0.0:4001", headers: "{\"Content-Type\":\"application/json\"}") {
  query: Query
  mutation: Mutation
  subscription: Subscription
}

Copy link

changeset-bot bot commented Apr 11, 2024

🦋 Changeset detected

Latest commit: fe69bff

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphql-tools/utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@enisdenjo enisdenjo mentioned this pull request Apr 11, 2024
14 tasks
Copy link
Contributor

github-actions bot commented Apr 11, 2024

✅ Benchmark Results

     ✓ no_errors
     ✓ expected_result

     checks.........................: 100.00% ✓ 336       ✗ 0  
     data_received..................: 39 MB   3.9 MB/s
     data_sent......................: 144 kB  14 kB/s
     http_req_blocked...............: avg=4.12µs   min=2.21µs   med=2.77µs   max=172.13µs p(90)=4.18µs   p(95)=4.76µs  
     http_req_connecting............: avg=656ns    min=0s       med=0s       max=110.21µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=55.36ms  min=47.37ms  med=51.86ms  max=155.58ms p(90)=58.62ms  p(95)=87.74ms 
       { expected_response:true }...: avg=55.36ms  min=47.37ms  med=51.86ms  max=155.58ms p(90)=58.62ms  p(95)=87.74ms 
     http_req_failed................: 0.00%   ✓ 0         ✗ 168
     http_req_receiving.............: avg=124.97µs min=101.23µs med=122.55µs max=287.82µs p(90)=137.35µs p(95)=146.45µs
     http_req_sending...............: avg=28.05µs  min=17.19µs  med=23.56µs  max=564.99µs p(90)=32.56µs  p(95)=36.51µs 
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=55.21ms  min=47.22ms  med=51.71ms  max=155.24ms p(90)=58.48ms  p(95)=87.58ms 
     http_reqs......................: 168     16.795292/s
     iteration_duration.............: avg=59.52ms  min=50.84ms  med=55.91ms  max=161.75ms p(90)=65ms     p(95)=91.76ms 
     iterations.....................: 168     16.795292/s
     vus............................: 1       min=1       max=1
     vus_max........................: 1       min=1       max=1

Copy link
Contributor

github-actions bot commented Apr 11, 2024

💻 Website Preview

The latest changes are available as preview in: https://12938a9d.graphql-tools.pages.dev

@enisdenjo enisdenjo changed the title isValidPath assumes non-paths are paths Disallow new lines in paths when checking with isValidPath Apr 12, 2024
@enisdenjo enisdenjo marked this pull request as ready for review April 12, 2024 17:26
Copy link
Contributor

github-actions bot commented Apr 12, 2024

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@graphql-tools/utils 10.1.3-alpha-20240412172754-fe69bff5da86ed3c142502448c9d7a0f5bd3a613 npm ↗︎ unpkg ↗︎

@enisdenjo enisdenjo requested a review from ardatan April 12, 2024 17:27
enisdenjo added a commit to ardatan/graphql-mesh that referenced this pull request Apr 12, 2024
@ardatan ardatan merged commit 4093f70 into master Apr 15, 2024
30 checks passed
@ardatan ardatan deleted the isvalidpath branch April 15, 2024 09:11
enisdenjo added a commit to ardatan/graphql-mesh that referenced this pull request Apr 15, 2024
enisdenjo added a commit to ardatan/graphql-mesh that referenced this pull request Apr 15, 2024
enisdenjo added a commit to ardatan/graphql-mesh that referenced this pull request Apr 16, 2024
enisdenjo added a commit to ardatan/graphql-mesh that referenced this pull request Apr 16, 2024
ardatan added a commit to ardatan/graphql-mesh that referenced this pull request Apr 16, 2024
* e2e setup

* prepare and setup

* cjs

* should start mesh

* improved utils

* available port and stuff

* use tsx cjs loader

* test compose and waitforexit reject

* fix compose

* e2e test ci

* chore(dependencies): updated changesets for modified dependencies

* sync getavailport

* e2e node matrix

* changeset

* serve script

* tenv and stuff

* tenv args and serve helper

* tenv only serve and compose

* esm config in cjs project

* simplify and use 0.0.0.0

* more details

* extend proc and less listners

* stderr for logs, stdout for outputs

* stable tenv std

* compose to stdout

* unnecessary serve script

* more wait for serve

* stable stderr

* compose to target

* simpler match for stability

* actually check stuff

* update snapshots

* snapshot file

* unnecessary port

* use 0.0.0.0

* unnecessary comment

* allow nodejs modules in e2e

* better args

* refactor and begin with type merging batching example

* increase timeout

* open example

* clarify

* link

* lol

* lol

* even more lol

* test plans

* WIP spawn detached and kill whole process group

* improve child process handling and use node with tsx for subgraphs

* stop reachability wait after exit

* append new line when logging to stderr

* nobuild e2e

* utils mkdir independant of fs

* type merging batching planning tests run concurrently

* execution tests

* check for aborted on retry

* touches

* fix(fusion/query-planner): skip the resolver if it has required variables that the parent subgraph doesn't have

* update snaps

* unnecessary assers

* listen to stderr

* subgraphs -> services

* service can be in <name>/index.ts

* thrift-calculator

* sqlite-chinook

* lockfile

* serveoptions

* ensure compose creates file

* tenv serve.execute

* just fusiongraph

* rest transport explicit type export

* openapi-javascript-wiki

* simpler doc

* tenv composition target is temp

* use target's absolute path if detected

* unused import

* increase reachability wait timeout even more

* try less workeser

* await available port making sure the server closes

* maxConcurr

* use __project

* wait for reachability longer

* disposable

* waitforexit is internal

* tenv containers and waiting adjustments

* mysql-employees

* mysql-employees no dates

* neo4j-example

* detectopenhandles

* args type leak from cjs-project

* auto-type-merging

* timeout e2e tests because of open handles

* batching-resolver

* federation-example

* unnecessary delay

* federation example servers

* soap-demo

* openapi-subscriptions

* no example queries

* revert prettierpath

* tfetch not necessary

* unnecessary deferstream plugin

* openapi v3 petstore

* mysql-rfam

* batching resolver simpler api service

* lockfile

* json-schema-subscriptions

* fusiongraph is not necessary

* json-schema-reddit

* neo4j uses serve and pubsub

* make sure the pubsub is destroyed

* mysql-rfam pubsub

* lockfile

* use ardatan/graphql-tools#6055

* chore(dependencies): updated changesets for modified dependencies

* specify endpoint for petstore

* auto-type-merging use container petstore

* use stable release of utils

* chore(dependencies): updated changesets for modified dependencies

* Fix federation example

* no skipping

* no dates in snaps

* Changeset

* update snap relating to a720512

* json-schema-reddit example

* use stable release of utils

* chore(dependencies): updated changesets for modified dependencies

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>
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