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

Remove trailing colon when parsing the protocol from URL #1778

Merged
merged 2 commits into from
Dec 21, 2022
Merged

Remove trailing colon when parsing the protocol from URL #1778

merged 2 commits into from
Dec 21, 2022

Conversation

wh201906
Copy link
Contributor

Motivation

This PR should fix #1775

Change description

Remove the trailing colon when parsing the protocol in URL string

Other information

Reviewer checklist

  • PR addresses a single concern.
  • The PR has no duplicates (please search among the Pull Requests before creating one)
  • PR title and description are properly filled.
  • Docs have been added / updated (for bug fixes / features)

@wh201906
Copy link
Contributor Author

I tested it on Windows and #1775 is fixed as expected.
I used Windows_X86-64_zip in the build artifacts for test.
https://github.com/arduino/arduino-ide/actions/runs/3718811226

@per1234 per1234 added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Dec 17, 2022
@wh201906
Copy link
Contributor Author

I have little experience of typescript. If my code style in this PR is not good, please tell me how to improve it. Thanks.

@kittaakos
Copy link
Contributor

The code changes look good to me. When I set and use the SOCKS scheme in the Settings > Network tab, I get the socks://username:pwd@hostname:port construct in the arduino-cli.yaml file.

I noticed the following, however:

Use these settings:

Screen Shot 2022-12-21 at 10 58 21

IDE2 will send the following config object to the CLI:

{
  "board_manager": {
    "additional_urls": [
      "https://arduino.esp8266.com/stable/package_esp8266com_index.json"
    ]
  },
  "daemon": {
    "port": "50051"
  },
  "directories": {
    "data": "/Users/a.kitta/Library/Arduino15",
    "user": "/Users/a.kitta/Documents/Arduino"
  },
  "library": {
    "enable_unsafe_install": false
  },
  "locale": "en",
  "logging": {
    "file": "",
    "format": "text",
    "level": "info"
  },
  "metrics": {
    "addr": ":9090",
    "enabled": true
  },
  "output": {
    "no_color": false
  },
  "sketch": {
    "always_export_binaries": false
  },
  "updater": {
    "enable_notification": true
  },
  "network": {
    "proxy": "http://username:pwd@alma:1234/"
  }
}

The updated CLI config will be this:

board_manager:
  additional_urls:
  - https://arduino.esp8266.com/stable/package_esp8266com_index.json
daemon:
  port: "50051"
directories:
  builtin:
    libraries: /Users/a.kitta/Library/Arduino15/libraries
  data: /Users/a.kitta/Library/Arduino15
  downloads: /Users/a.kitta/Library/Arduino15/staging
  user: /Users/a.kitta/Documents/Arduino
library:
  enable_unsafe_install: false
locale: en
logging:
  file: ""
  format: text
  level: info
metrics:
  addr: :9090
  enabled: true
network:
  proxy: socks://username:pwd@my_hostname:3636
output:
  no_color: false
sketch:
  always_export_binaries: false
updater:
  enable_notification: true

When I use the HTTP scheme in the Settings > Network tab:

Screen Shot 2022-12-21 at 11 04 18

The IDE2 will send the following config JSON to the CLI:

{
  "board_manager": {
    "additional_urls": [
      "https://arduino.esp8266.com/stable/package_esp8266com_index.json"
    ]
  },
  "daemon": {
    "port": "50051"
  },
  "directories": {
    "data": "/Users/a.kitta/Library/Arduino15",
    "user": "/Users/a.kitta/Documents/Arduino"
  },
  "library": {
    "enable_unsafe_install": false
  },
  "locale": "en",
  "logging": {
    "file": "",
    "format": "text",
    "level": "info"
  },
  "metrics": {
    "addr": ":9090",
    "enabled": true
  },
  "network": {
    "proxy": "http://u:p@host:2345/"
  },
  "output": {
    "no_color": false
  },
  "sketch": {
    "always_export_binaries": false
  },
  "updater": {
    "enable_notification": true
  }
}

But the CLI config YAML will be this:

board_manager:
  additional_urls:
  - https://arduino.esp8266.com/stable/package_esp8266com_index.json
daemon:
  port: "50051"
directories:
  builtin:
    libraries: /Users/a.kitta/Library/Arduino15/libraries
  data: /Users/a.kitta/Library/Arduino15
  downloads: /Users/a.kitta/Library/Arduino15/staging
  user: /Users/a.kitta/Documents/Arduino
library:
  enable_unsafe_install: false
locale: en
logging:
  file: ""
  format: text
  level: info
metrics:
  addr: :9090
  enabled: true
network:
  proxy: http://u:p@host:2345/
  user_agent_ext: daemon
output:
  no_color: false
sketch:
  always_export_binaries: false
updater:
  enable_notification: true

👆 Note the additional network@user_agent_ext: daemon entry. I could not consistently reproduce it.


I have also noticed when switching from either SOCK or HTTP proxy to no proxy config, the CLI config does not update correctly. The network prop remains in the YAML.

Have this Settings > Network tab:

Screen Shot 2022-12-21 at 11 09 53

IDE2 sends this CLI config JSON to CLI:

{
  "board_manager": {
    "additional_urls": [
      "https://arduino.esp8266.com/stable/package_esp8266com_index.json"
    ]
  },
  "daemon": {
    "port": "50051"
  },
  "directories": {
    "data": "/Users/a.kitta/Library/Arduino15",
    "user": "/Users/a.kitta/Documents/Arduino"
  },
  "library": {
    "enable_unsafe_install": false
  },
  "locale": "en",
  "logging": {
    "file": "",
    "format": "text",
    "level": "info"
  },
  "metrics": {
    "addr": ":9090",
    "enabled": true
  },
  "network": {},
  "output": {
    "no_color": false
  },
  "sketch": {
    "always_export_binaries": false
  },
  "updater": {
    "enable_notification": true
  }
}

But the CLI YAML will be this:

board_manager:
  additional_urls:
  - https://arduino.esp8266.com/stable/package_esp8266com_index.json
daemon:
  port: "50051"
directories:
  builtin:
    libraries: /Users/a.kitta/Library/Arduino15/libraries
  data: /Users/a.kitta/Library/Arduino15
  downloads: /Users/a.kitta/Library/Arduino15/staging
  user: /Users/a.kitta/Documents/Arduino
library:
  enable_unsafe_install: false
locale: en
logging:
  file: ""
  format: text
  level: info
metrics:
  addr: :9090
  enabled: true
network:
  proxy: http://uname:korte@hostname:4567/
  user_agent_ext: daemon
output:
  no_color: false
sketch:
  always_export_binaries: false
updater:
  enable_notification: true

The network prop must be missing or empty. It's probably a CLI bug, as IDE2 never writes the config file.

@kittaakos kittaakos self-requested a review December 21, 2022 10:10
Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

I have verified the changes and confirm that it fixes #1775. When I configure the socks protocol in the Settings > Network tab, I have the expected socks://username:pwd@hostname:port construct in the ~/.arduinoIDE/arduino-cli.yaml file.

Thank you for the help!

@kittaakos
Copy link
Contributor

IDE2 will send the following config object to the CLI:

FYI: If one starts the IDE2 from the console, the actual CLI config JSON object is dumped when saving the Settings dialog and updating the CLI config through the daemon.

@wh201906
Copy link
Contributor Author

I guess we need to start a new issue to resolve the problems you find in #1778 (comment)?

@wh201906
Copy link
Contributor Author

FYI: If one starts the IDE2 from the console, the actual CLI config JSON object is dumped when saving the Settings dialog and updating the CLI config through the daemon.

It's helpful. Thanks.

@kittaakos
Copy link
Contributor

I guess we need to start a new issue to resolve the problems you find in #1778 (comment)?

Thanks for your prompt reaction. Yes, we will take care of the follow-ups, and it does not affect this PR. No action is required (unless you want to start working on the other bugs 😉, but they need to be confirmed first.)

@kittaakos kittaakos merged commit 644e607 into arduino:main Dec 21, 2022
@wh201906 wh201906 deleted the fix-proxy-parser branch December 21, 2022 13:33
@per1234
Copy link
Contributor

per1234 commented Dec 21, 2022

I have also noticed when switching from either SOCK or HTTP proxy to no proxy config, the CLI config does not update correctly. The network prop remains in the YAML.

This bug is tracked at arduino/arduino-cli#1677

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

"Preferences" dialog shows proxy is configured as SOCKS after user configures HTTP proxy
3 participants