8000 refactor: Parsing adr poc url by ashearin · Pull Request #259 · bomctl/bomctl · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

refactor: Parsing adr poc url #259

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

refactor: Parsing adr poc url #259

wants to merge 5 commits into from

Conversation

ashearin
Copy link
Member
@ashearin ashearin commented Jan 9, 2025 •
edited
Loading

Description

Proof of concept for 2 items:

  • unified cmd url parsing
  • using net/url parsing to remove custom parsing code and rely on standard format

The URL standard is the closest to what bomctl currently uses and was the logical first choice for a Proof of Concept. The command URL is largely the same as currently utilized, however any custom elements are being removed and moved into a url compliant field. example below reworks this command url in the currently implemented format:
- https://username:password@github.com:12345/bomctl/bomctl.git@main#sbom.cdx.json

URL: [scheme:][//[userinfo@]host][/]path[?query][#fragment]

https://username:password@github.com:12345/bomctl/bomctl.git?ref=main#sbom.cdx.json

scheme: https
userinfo: [username, password]
host: github.com:12345 (parse port with url.Port())
path: bomctl/bomctl.git
fragment: sbom.cdx.json
Query: ref=main (parse query with url.Query() and get value with query.Get("ref"))

Main difference is the @main used to designate the branch is moved into the query field. Another option is removing it entirely and creating a branch flag and defaulting to the default branch if a branch is not provided (although this may only be applicable to the git client)

Optional improvements to what is changed, specifically in OCI client: (Can be implemented in separate issues)

  • Init function for each client that validates the given URL (Implemented)
  • Init function could also:
    • Replace functionality common to prepareFetch and preparePush
    • Ensure needed infrastructure is initialized, like createRepository in OCI client
  • Store target url in client implementation structs to avoid repeated url parsing and reduce function parameters
    • Store target ref in a similar fashion
  • In clients that utilizes (the git flavored ones), do not require the branch to be passed in and assume the user intends the default branch if it's not given by a user
    • potentially make a flag to specify a different flag

@ghost
Copy link
ghost commented Jan 9, 2025

Minder Vulnerability Report ✅

Minder analyzed this PR and found it does not add any new vulnerable dependencies.

Vulnerability scan of bae61b1c:

  • 🐞 vulnerable packages: 0
  • 🛠 fixes available for: 0

@ashearin ashearin force-pushed the parsing-adr-poc-url branch from d83be63 to bf37dd5 Compare January 10, 2025 03:07
@ashearin ashearin force-pushed the parsing-adr-poc-url branch from bf37dd5 to 8cebe63 Compare January 10, 2025 03:45
ashearin
ashearin commented Jan 13, 2025
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@ashearin
0