Skip to content

fix: help to handle @ when it is not encoded in url#7609

Merged
ihexxa merged 2 commits intodevelopfrom
fix/url-invalid-char
Jun 28, 2024
Merged

fix: help to handle @ when it is not encoded in url#7609
ihexxa merged 2 commits intodevelopfrom
fix/url-invalid-char

Conversation

@ihexxa
Copy link
Copy Markdown
Contributor

@ihexxa ihexxa commented Jun 28, 2024

Background

Normally @ is one URL separator and should be encoded if it used in path, but users normally don't aware of this, which will cause url parsing fail.

Changes

  • fix: help to handle @ when it is not encoded in url
  • added UTs

Ref: INS-4083, #7603

@ihexxa ihexxa self-assigned this Jun 28, 2024
static parse(urlStr: string): UrlOptions | undefined {
// the URL API (for web) is not leveraged here because the input string could contain tags for interpolation
// which will be encoded, then it would introduce confusion for users in manipulation
// TODO: but it still would be better to rely on the URL API
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is a good idea, perhaps try it here in the bugfix pr

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is true, I tried several times so I added comment here :). As tags, path params could be in it, URL will reject most of test cases. Let's merge it and I'll show this in the next.

@ihexxa ihexxa merged commit cb4ec74 into develop Jun 28, 2024
@ihexxa ihexxa deleted the fix/url-invalid-char branch June 28, 2024 13:03
stefancruz pushed a commit to stefancruz/insomnia that referenced this pull request Jun 30, 2024
* fix: help to handle @ when it is not escaped in url

* test: add tests and comments
CurryYangxx pushed a commit that referenced this pull request Jul 5, 2024
* fix: help to handle @ when it is not escaped in url

* test: add tests and comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants