Repository navigation
Quotes in header values break almost all snippets for all Chromium HARs #271
Description
For example, I'm pretty sure this is an entirely valid raw HTTP header:
My-Header: "quoted" 'value'
In almost all cases I've seen, httpsnippet inserts this header value literally, without escaping either quote format, but within quotes in the output syntax, and so generating broken output every time, like: headers: { "My-Header": ""quoted" 'value'" }
This is bad because quotes are used in a lot of real HTTP traffic, for example new Chromium versions now send a "sec-ch-ua-platform" header on every single HTTPS request, and the value is always wrapped in quotes: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-CH-UA-Platform.
That means this breaks almost all snippets from almost all Chrome HAR files, making them all syntactically invalid.
The only cases I've seen where this does work is the JS snippets that use stringify-object to build an options object, which generates single-quoted output and automatically escapes any included string quotes en route.
Activity
pimterry commentedon Jun 8, 2022
I've actually been using my own httpsnippet fork, since the repo seemed abandoned, and in my fork (based on the pre-typescript code unfortunately) @eeWynell added a %q format to the codebuilder that handles quotes automatically, which might be interesting: httptoolkit/httpsnippet@2401900#diff-78e5fb011bd79a6d7b6f88df468aadeac95cf9c8adc86268642815b77f8bfaf2.
That was intended to start fixing some similar issues and add a base to add other new languages, but something similar might help solve this issue here.
As an aside - I'm excited to see the many changes here recently! I'd be very happy to drop my fork and migrate back to the main package, if there's a chance that it'll be actively maintained in future. I'm using httpsnippet in HTTP Toolkit. I'd be happy to help with the maintenance work here if there's any way I can contribute, since I'm using it heavily and currently maintaining my own fork of the same thing anyway, just let me know.
pimterry commentedon Jun 27, 2022
@filfreire @dimitropoulos @erunion Any thoughts on the above?
This breaks httpsnippet for a something like 70% of modern browser traffic right now (all Chromium-based traffic). That sec-ch-ua-platform header is widely used, it always contains quotes in the value, and the quotes are never escaped in snippets, so they all break. It'd be great to get that fixed.
pimterry commentedon Jul 1, 2022
I've now fixed this on my fork, here: httptoolkit/httpsnippet@a107032. If anybody else is running into this issue, you can use @httptoolkit/httpsnippet fork@2.1.1+ to fix this.
If this repo is being maintained in future, you might find the changes in that commit useful, especially the tests, since I think it has corrected output examples for both types of quote, for every single snippet. Some are not as easy as you think - you need to escape different quotes depending on the quote types used in each language, and there's lots of odd edge cases (e.g. powershell, uses ` instead of \ for escaping, all shell clients escape by swapping quote types and implicitly concatenating, etc).
I think the implementation there handles this all correctly (and certainly, it passes the tests). It'll be easy for you to re-use those tests if you like but a bit more complicated to integrate the fixes I expect, since my fork predates the recent TypeScript rewrite, but you can get the gist at least.
Again: I'd really recommend fixing this ASAP, because it breaks httpsnippet right now for an enormous percentage of use cases with modern tools (70% of browser traffic!).
I'd still be very happy to combine forces instead of maintaining this fork totally independently, just let me know if there's a way I can usefully do so.
dimitropoulos commentedon Jul 13, 2022
(sorry: was PTO for a few days) Definitely super interested in what you're talking about! As you can see, quite a few PRs have been merging right along here lately, so I would definitely welcome any from you.
In fact, it's my goal to have 0 open PRs. I think we really can do it!
Do you have any idea for porting this fix into this repo? We're not going to go back to the utils.format package.
In short: yes! let's combine forces! Where can we start?
pimterry commentedon Jul 15, 2022
The test cases can be ported directly fairly easily I think, so that'd be a good start.
From there you can take the same logic as the format helper here, but just including the base
escapefunction from the above commit as a standalone utility function and calling it inline, much like how some examples use things like stringifyObject. Would that work for you?dimitropoulos commentedon Jul 15, 2022
yes! are you able to start the work, maybe the porting of the tests part?
dimitropoulos commentedon Jul 28, 2022
fixed by #289 (comment)