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

fix: query parameters containing a colon (:) are not parsed correctly when changed #3045

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pietrygamat
Copy link
Contributor

@pietrygamat pietrygamat commented Sep 5, 2024

Description

After saving the request, the colon (:) in a parameter name is being consumed by bru parser and what follows is treated as parameter value - so there must be a way to allow either quoting bru keys, or escaping problematic characters. In this PR I introduce a way to store dictionary keys for query params quoted. The quotes are discarded when loading collections, so they will be invisible in the UI.
fixes #3037
fixes #2810 (alternative to #2898)
fixes #2878 (alternative to #2898)

Automatic quoting is currently only applied when saving query parameters. It does not make a lot of sense for request headers, where colon is disallowed by HTTP spec. It may make sense to add in multipart-form body key-value table and vars table (#3178).

Example

Screencast.from.2024-09-14.00-41-20.mp4
params:query {
  normal-param: value
  "sort:field": name
  ~"sort:order": DESC
  ~"\"name\"\"\"": Dark
}

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

@pietrygamat pietrygamat force-pushed the bugfix/3037 branch 2 times, most recently from 9c718da to d7465f0 Compare September 5, 2024 19:18
@pull-request-size pull-request-size bot added size/M and removed size/S labels Sep 13, 2024
@pietrygamat pietrygamat force-pushed the bugfix/3037 branch 2 times, most recently from 9a160ff to 24a633f Compare September 13, 2024 23:06
@@ -4,6 +4,10 @@ const { indentString } = require('../../v1/src/utils');

const enabled = (items = []) => items.filter((item) => item.enabled);
const disabled = (items = []) => items.filter((item) => !item.enabled);
const quoteKey = (key) => {
const quotableChars = [':', '"', '{', '}', ' '];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The characters

:  {  } "

are important for bru syntax, and so they should be escaped by default. The question of space is open for discussion, as it may be solved in a different way (#2898) without forcing changes onto users' bru files.

… names query parameters

Automatically quote query parameter keys if they contain characters sensitive for bru syntax: ':', '"', '{', '}' or ' '.
Quoted keys stored in .bru files now support escaping, so it is possible to store keys that themselves contain '"' character.

Fixes usebruno#3037
Fixes usebruno#2810
Fixes usebruno#2878
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants