8000 refactor(pkg/xurl): change protocol functions to prevent possible bugs by jeronimoalbi · Pull Request #2392 · ignite/cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

refactor(pkg/xurl): change protocol functions to prevent possible bugs #2392

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

Merged
merged 10 commits into from
Apr 28, 2022

Conversation

jeronimoalbi
Copy link
Member
@jeronimoalbi jeronimoalbi commented Apr 19, 2022

fixes #2390

Description

The protocol related functions should be refactored to avoid errors:

  • The URI value should not be empty
  • When the scheme exists it must be the one ensured by the function
  • When a different scheme exists it must be replaced by the ensured one
  • Protocol functions must be unit tested
  • The code using these functions must handle any parse error

The "TestHTTPEnsurePort" was slightly changed to be consistent with the
existing table driven tests conventions.
@jeronimoalbi jeronimoalbi added type:error Something isn't working type:refactor labels Apr 19, 2022
@jeronimoalbi jeronimoalbi self-assigned this Apr 19, 2022
@ilgooz ilgooz marked this pull request as ready for review April 20, 2022 18:07
ivanovpetr
ivanovpetr previously approved these changes Apr 21, 2022
ivanovpetr
ivanovpetr previously approved these changes Apr 22, 2022
@jeronimoalbi jeronimoalbi marked this pull request as draft April 22, 2022 14:16
The protocol related functions now check for values like for example
0.0.0.0:42 or localhost:42 that can't be parsed by the net package
because the scheme is not defined.
@jeronimoalbi jeronimoalbi marked this pull request as ready for review April 22, 2022 16:18
Copy link
Member
@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member
@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

Great addition with tests, thank you for the detailed inline comments! 👏

@ilgooz ilgooz merged commit 272673e into develop Apr 28, 2022
@ilgooz ilgooz deleted the refactor/pkg-xurl branch April 28, 2022 17:17
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
ignite#2392)

* refactor(pkg/xurl): add error support to protocol functions

* refactor(pkg/xurl): change existing ensure port test for consistency

The "TestHTTPEnsurePort" was slightly changed to be consistent with the
existing table driven tests conventions.

* refactor: change calls to xurl protocol functions to handle errors

* refactor(pkg/xurl): add constants for scheme types

* fix: formatting

* fix: formatting

* fix(pkg/xurl): change ensure protocol function to handle ADDR:HOST

The protocol related functions now check for values like for example
0.0.0.0:42 or localhost:42 that can't be parsed by the net package
because the scheme is not defined.

* chore: review improvements

* feat(pkg/xurl): add MightHTTPS function

* chore: change network publish command to ensure http/https
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:error Something isn't working type:refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor pkg/xurl
3 participants
0