Escape quotes in headers correctly in all languages #289

pimterry · 2022-07-18T18:00:15Z

As discussed in #271 (comment) - this ports the test changes from https://github.com/httptoolkit/httpsnippet into the new folder structure here, providing working examples for correct quote-escaping for every single language/format.

~~This does not make the tests pass, we'll need to do that separately - currently this fails for 22 of the snippets, where they're currently not escaping quotes right now.~~

I think the base logic to fix this should look something like this, which we'll need to call in the appropriate places with the appropriate options (delimiter, escapeChar and escapeNewlines). You can see how it's used for each snippet in the original commit to check which ones you need, but the tests here should be enough to confirm that's correct.

~~Happy for somebody else to look into fixing this, if anybody has time, or I should be able to migrate the fix for this myself late this week or early next if not.~~

EDIT: Now updated to add a working implementation, in addition to updating all the test fixtures to cover this.

dimitropoulos · 2022-07-18T20:03:23Z

wow thanks! sounds like you can see we're catching up around here! :)

Still working on the readme, but there's a super useful shortcut I wanted to tell you about: OVERWRITE_EVERYTHING=true npm run test. Like it sounds, haha, it will overwrite all tests. It's useful when working on a change that will update fixtures.

pimterry · 2022-07-27T10:35:27Z

All done, this now fixes #271, and there's also a tiny patch in here for weird indentation around python3 header values.

src/helpers/escape.ts

src/targets/python/python3/client.ts

src/helpers/escape.test.ts

dimitropoulos

@erunion could you please take a look? otherwise, I'm happy with this!

@pimterry you're the first brave soul to update any of the root fixtures! thanks! If there is anything about the process that was needlessly difficult for you, please let me know. :)

* Total Overhaul (but with all the same fixtures!) (#248) see Kong#248 and the commits therein for more context. Essentially: - the original client fixtures were unchanged, which hopefully means the - all source code is now in strict mode TypeScript - tests are now all in jest - the file structure was reorganized so that everything for a particular client is in one place - the CLI is updated and now using yargs - all dependencies were updated and some (i.e. `format.utils`) were able to be removed entirely - more work left to do (including CI with GitHub Actions, for example), but this is a start * fix: case where if `postData.params` is missing some targets crash (#258) Co-authored-by: Dimitri Mitropoulos <[email protected]> * fix: compatibility issues on node 14 with `Object.hasOwn()` (#252) * fix: typo in the httpie `style` option not being correctly applied (#254) Co-authored-by: Dimitri Mitropoulos <[email protected]> * fix: axios targets not sending `x-www-form-urlencoded` properly (#255) Co-authored-by: Dimitri Mitropoulos <[email protected]> * feat: addition of a PHP target for Guzzle (#253) Co-authored-by: Dimitri Mitropoulos <[email protected]> * Add Github Build Workflow (#250) (#251) * Add Github Build Workflow (#250) * Edit job name * Replace install with ci on GH workflow * Add matrix with major node versions (14, 16, 18) * Disable fail-fast * Remove node v14 from build GH action * feat: native upload support in python `requests` snippets (#259) Co-authored-by: Dimitri Mitropoulos <[email protected]> * fix: `multipart/form-data` header issues with node/js fetch targets (#257) Co-authored-by: Dimitri Mitropoulos <[email protected]> * fix: headers not being properly applied to R httr snippets (#263) Co-authored-by: Dimitri Mitropoulos <[email protected]> * Fix build workflow dispatch rules (#265) * Chore: Remove travis links (#266) * Remove travis links * Update README.md Co-authored-by: Dimitri Mitropoulos <[email protected]> * fix: issue where query strings in R wouldn't be properly concatenated (#269) * fix: issue where query strings in R wouldn't be properly concatenated * adds (and respects) indent options to httr, plus double looping fix now, indent is respected, and also avoiding running Object.keys twice per run since we can just run it once with .entries Co-authored-by: Dimitri Mitropoulos <[email protected]> * add header namesspace to prevent header errors (#247) * add header namesspace to prevent header errors * update fixtures Co-authored-by: Dimitri Mitropoulos <[email protected]> * fix: stop implicitly coercing warning in Swift snippet generator (#195) * swift/nsurlsession adds `as Any` to print for error * adds OVERWRITE_EVERYTHING to ease fixture snapshot resetting * updates fixtures Co-authored-by: Dimitri Mitropoulos <[email protected]> * fix: clj-http handling of literal null JSON bodies (#283) Co-authored-by: Sergey Zakharchenko <[email protected]> * fix: prevent crash in Swift/Objc with checking length of input body post params (#192) Co-authored-by: Dimitri Mitropoulos <[email protected]> * fix: cUrl target should encode x-www-form-urlencoded post data params (#198) Co-authored-by: Dimitri Mitropoulos <[email protected]> * feat: Add support for insecureSkipVerify (#285) * go/native: adds insecureSkipVerify * node/native: adds insecureSkipVerify * python/python3: adds insecureSkipVerify * ruby/native: adds insecureSkipVerify * shell/curl: adds insecureSkipVerify Co-authored-by: Tim Perry <[email protected]> * feat: implementing cleaner handling of JSON in cURL snippets (#256) Co-authored-by: Dimitri Mitropoulos <[email protected]> * chore: minor cleanup (#286) * feat: use curl's --compressed option for requests that accept it (#287) * feat: make Python snippets simpler, clearer & more consistent (#288) Co-authored-by: Tim Perry <[email protected]> * feat: change the default response code for Python Requests (#181) Co-authored-by: Dimitri Mitropoulos <[email protected]> * feat: PHP JSON body encoding (#291) * php/curl: use json_encode for CURLOPT_POSTFIELDS * php/http1: use json_encode when body is JSON * php/http2: use json_encode when body is JSON Co-authored-by: Andrii Kostenko <[email protected]> * Async/Await (top level) support in JavaScript snippets (#292) * Exclude package.json from build to fix output paths (#294) * Exclude package.json from build to fix output paths * keeps bin pointing at cli output * makes rootDir explicit * removes unused cli build scripts Co-authored-by: Dimitri Mitropoulos <[email protected]> * Fix crash when building nsurlsession snippets for empty params (#295) * removes `require 'openssl'` from ruby target (no longer needed) (#296) * Escape quotes in headers correctly in all languages (#289) * updates README (#299) Co-authored-by: Filipe Freire <[email protected]> * ioutil -> io (deprecated) (#305) * chore: undoing unwanted changes * chore: revert more unwanted changes * chore: reverting more unwanted changes * fix: fixing broken test snapshots and libcurl not escaping * fix: a bunch of broken tests * fix: removing dead code * fix: remove support for `insecureSkipVerify` as we dont need or want it * fix: removing top-level await changes for axios * fix: revert top-level await changes for js:fetch * fix: remove some problematic changes to `node:request` * fix: retaining line trimming in powershell snippets * fix: bug in php snippets where booleans were casted to null * fix: revert problematic changes to python:requests * fix: revert more unwanted changes * chore: temporarily skipping the integration suite * fix: broken snapshot * fix: disabling the integration suite from being run without inside docker * fix: integration suite * fix: integration suite --------- Co-authored-by: Dimitri Mitropoulos <[email protected]> Co-authored-by: Filipe Freire <[email protected]> Co-authored-by: Davis Martin <[email protected]> Co-authored-by: iraj taghlidi <[email protected]> Co-authored-by: Sergey Zakharchenko <[email protected]> Co-authored-by: Julien Giovaresco <[email protected]> Co-authored-by: Tim Perry <[email protected]> Co-authored-by: Andrii Kostenko <[email protected]> Co-authored-by: Tim Perry <[email protected]> Co-authored-by: Alexander Weber <[email protected]>

After version 107, restsharp completely changed their API. This PR fixes the code generator to support the new API. - also add the escaping function from Kong#289 - I think there are probably still escaping bugs here, but it seems to be working Well Enough™

pimterry added 3 commits July 27, 2022 12:08

Add string escaping helper method

b4d7bda

Escape quotes appropriately in headers values in all snippets

e659f4c

Fix tiny Python snippet indentation issue

03bee2f

pimterry force-pushed the quote-tests branch from 6742c4e to e659f4c Compare July 27, 2022 10:15

pimterry changed the title ~~Add (failing!) tests for quote handling for all languages~~ Escape quotes in headers correctly in all languages Jul 27, 2022

pimterry marked this pull request as ready for review July 27, 2022 10:35