graphqurl icon indicating copy to clipboard operation
graphqurl copied to clipboard

Update header parsing to allow double-escaped ":"

Open porkloin opened this issue 3 years ago • 4 comments

What Does This PR Do?

  • Modifies header parsing logic to leverage regex instead of basic split

Why?

Currently, any header value that contains literal colon characters is blanket rejected by these lines of code. My first impulse as a user was to try escaping those characters. However, I quickly realized that the logic didn't support escaping chars either. This regex split should only select colon characters not preceded by a double escape. Double escape must be used instead of single escape, since JS' strings will infer a regular escape as a standard escape char for the string system and store it without those escape chars.

Questions to Think About :thinking:

Negative lookbehind support (which this regex uses) isn't great across older JS versions - I believe ES2018 was when support was added. If compatibility is an issue let me know, and I can probably put together an alternative approach that still allows escaping these chars.

porkloin avatar Mar 09 '21 17:03 porkloin

Beep boop! :robot:

Hey @porkloin, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible.

Stay awesome! :sunglasses:

hasura-bot avatar Mar 09 '21 17:03 hasura-bot

Hey! Any thinks on this one? I know you folks are probably busy, but I'm more than happy to put something together that meets whatever reqs you might have for this. I regularly work with some headers that I have to forward to my GQL endpoints that unfortunately contain literal : and I don't have the ability to negotiate on the structure of the header. I'd love to be able to use this tool for that work!

porkloin avatar Apr 15 '21 02:04 porkloin

Simple enough change to allow double quotes.

threeiem avatar Jul 08 '21 03:07 threeiem

I've recently met this issue and I'd like to suggest another apporach: instead of supporting colon escaping just allow colons in header value by joining by colon parts after the first : headerValue = parts.slice(1).join(':'). Also it would be useful to change parts[1].trim() below to headerValue.trimStart() to preserve possibly intentional spaces in the end of header value.

Edit: forgot that in JS split works differently from Python.

daa avatar Oct 07 '21 20:10 daa