graphql-inspector icon indicating copy to clipboard operation
graphql-inspector copied to clipboard

Some headers are incorrectly parsed

Open aaronleopold opened this issue 2 years ago • 5 comments

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 pnpm generate in the terminal.

    Please make sure the Codegen and plugins version 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

I am trying to add an Origin header to command, but my server is picking up an incorrect header. For example, if I run:

graphql-inspector introspect http://localhost:5500/api --header 'Origin: http://localhost:5500/api' --write schema.graphql

The header value that gets passed to my server is almost correct:

http//localhost:5500/api

I believe the issue is here: https://github.com/kamilkisiela/graphql-inspector/blob/master/packages/commands/commands/src/index.ts#L51-L55

To Reproduce Steps to reproduce the behavior:

Expected behavior

Environment:

  • OS: macOS
  • @graphql-inspector/cli: ^4.0.2
  • graphql: ^16.8.1
  • NodeJS: v18.10.0

Additional context

Here is a basic reproduction using the linked logic above:

https://www.typescriptlang.org/play?#code/MYewdgzgLgBAhgJwOYRgXhgbwFAzzACwFM4ATIhALhgG1d8GAiAeQQEsk2xqCooAHSgHohAGxDA4ogiGiUArIvmMANPTwBdbAF9s2UUVjEyFVBkzb4qAEpFQCUgB5o7MEhUwXXJAD4A3HoAZiAIMAAUoJBGJOShIIHwyBAAdMaxAJRY6jCR0LRgcAC2RB7JZQBuUgCuRBAa6IQxFMkQ-KJsUGEA5JRd6QHZaaY0BcX1GJWiNSkAViBc3X0Buti5IAbJ4khhQwgQ6XoGsEggIKQAEk17DRZWMLb2Tl5uHs++A8GhEeB5uzDxiRQqSumRwDFysBGRRKMDKyUm03GjRMCBabQ63V6-T0DBOZ0uKIgULGDQRtWShTg-DCYRcmTQPk8UFRzLYhTC6XSyTmCx6Sx0ejWGy2YTxFyu+yAA

aaronleopold avatar Nov 15 '23 17:11 aaronleopold

Thanks for reporting @aaronleopold !

Sorry but I'm not adding a lot here but just labeling it according to our new Contribution Guide and issue flow.

It seems already got into stage 1 thanks to your reproduction! Thank you for that!

Now in order to advance to stage 2 we'll need a failing test, do you think you are able to create one?

Thank you and sorry that this comment is not a complete solution (yet).

Urigo avatar Nov 19 '23 16:11 Urigo

Thanks for reporting @aaronleopold !

Sorry but I'm not adding a lot here but just labeling it according to our new Contribution Guide and issue flow.

It seems already got into stage 1 thanks to your reproduction! Thank you for that!

Now in order to advance to stage 2 we'll need a failing test, do you think you are able to create one?

Thank you and sorry that this comment is not a complete solution (yet).

Hey 👋 no worries!

When you ask for a failing test, are you asking for a code change that literally adds a failing test somewhere? Sorry if that seems an obvious question haha just want to clarify. If so, I can probably find time next weekend to do so. However, my playground link has a tentative fix underneath the reproduction, so at that point if I'm already issuing a PR would it just be preferred to make the fix directly?

aaronleopold avatar Nov 19 '23 22:11 aaronleopold

Yes exactly. Even if you already have the solution, we always try to include a test case with our solutions so we'll make sure it won't happen again. The best case is to submit a PR with a failing test, see that it actually fails on CI and then push another commit with the fix. Thank you so much for you help!

Urigo avatar Nov 20 '23 22:11 Urigo

Hello, we have the same errors and after some debug we understand that it was on graphql-inspector and we found this issue created. Any way to help on this issue to make it fixed quickly ? :smiley:

AdelELHAIBA-Akeneo avatar Dec 05 '23 14:12 AdelELHAIBA-Akeneo

Any way to help on this issue to make it fixed quickly ? 😃

Sorry this fell off my radar since I didn't have time to wait for the outlined process because of a work requirement (and try not to work too much on the weekends 😅). I'll try to create the PR with the failing CI and followup with a correcting commit, but if you (@AdelELHAIBA-Akeneo) need the fix sooner my reproduction had a tentative fix you can use if you have the time to do what @Urigo outlined above

aaronleopold avatar Dec 05 '23 14:12 aaronleopold