Skip to content

Fixed panic if malformed 3rd party url is specified #2817

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 20, 2025

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Jan 20, 2025

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

Fix the panic reported in #2786

What is the current behavior?

The arduino-cli panics by reproducing the steps described in #2786

What is the new behavior?

$ arduino-cli update-index
Error initializing instance: Invalid additional URL: parse "foo=https://arduino.esp8266.com/stable/package_esp8266com_index.json": first path segment in URL cannot contain colon
Downloading index: package_index.tar.bz2 downloaded                                                                                                                           
Downloading index: foo=https://espressif.github.io/arduino-esp32/package_esp32_index.json Unable to parse URL: parse "foo=https://espressif.github.io/arduino-esp32/package_esp32_index.json": first path segment in URL cannot contain colon
Some indexes could not be updated.
$

Does this PR introduce a breaking change, and is titled accordingly?

Other information

Fix #2786

@cmaglie cmaglie added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Jan 20, 2025
@cmaglie cmaglie self-assigned this Jan 20, 2025
@cmaglie cmaglie marked this pull request as draft January 20, 2025 09:23
Copy link
Contributor

@alessio-perugini alessio-perugini left a comment

Choose a reason for hiding this comment

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

changes LGTM. After we rebased this onto #2819 the CI should be 🟢

@cmaglie cmaglie marked this pull request as ready for review January 20, 2025 10:38
@cmaglie cmaglie merged commit e9092cc into arduino:master Jan 20, 2025
97 checks passed
@cmaglie cmaglie deleted the fix_panic branch January 20, 2025 10:43
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 67.71%. Comparing base (f47399b) to head (e8e9a31).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
commands/instances.go 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2817      +/-   ##
==========================================
+ Coverage   67.62%   67.71%   +0.08%     
==========================================
  Files         238      238              
  Lines       22388    22388              
==========================================
+ Hits        15140    15159      +19     
+ Misses       6050     6034      -16     
+ Partials     1198     1195       -3     
Flag Coverage Δ
unit 67.71% <77.77%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic on invalid board_manager.additional_urls config value
2 participants