AddBroker: Escape % in Addresses #319
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The url.Parse(server) will fail if given an address containing a non escaped % which is the case for an IPv6 address with a zone indication. Ex. [fe80::a604:5838:8d8a:e532%wlp58s0]:1883 => [fe80::a604:5838:8d8a:e532%25wlp58s0]:1883 Regexp explanation: `%(25)?` % - Match % literally (case sensitive) (25)? - Match 25 literally between zero and one time. => Will match any % followed by 25 or not. The Regexp.ReplaceAll(...) will replace all matches of the regular expression above and replace them with %25. Which is the URL escape code for %. Signed-off-by: Måns Ansgariusson <[email protected]>
Contributor
|
Thanks |
Contributor
|
Thought I should add a note to this commit so relevant parties are notified. I am proposing to undo this change because it is causing issues elsewhere (when using web sockets the URL may contain % and escaping this breaks AWS auth). Given that RFC6874 states that % must be escaped I believe the best approach is to undo this change and leave it up to the caller to provide an address that complies with the standard. See issues #469 and #479 for further info. |
MattBrittan
added a commit
to ChIoT-Tech/paho.mqtt.golang
that referenced
this pull request
Jan 28, 2021
…r string). This change broke connections to AWS (and potentially other websocket providers) due to the use of parameters. Ref issues eclipse-paho#479 and eclipse-paho#469. Signed-off-by: Matt Brittan <[email protected]>
MattBrittan
added a commit
that referenced
this pull request
Jan 28, 2021
Remove changes introduced in PR #319 (escaping % in server string). This may break some apps using IPV6 and passing in a server string that does not comply with RFC6874 (unescaped % in zone identifier). However the original change breaks more use cases so removing it is the best available option.
Contributor
|
This change has now been reversed in @master. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The url.Parse(server) will fail if given an address containing a
non escaped % which is the case for an IPv6 address with a zone
indication.
Ex.
[fe80::a604:5838:8d8a:e532%wlp58s0]:1883 => [fe80::a604:5838:8d8a:e532%25wlp58s0]:1883
Regexp explanation:
%(25)?% -- Match % literally (case sensitive)
(25)? -- Match 25 literally between zero and one time.
=> Will match any % followed by 25 or not.
The Regexp.ReplaceAll(...) will replace all matches of the regular
expression above and replace them with %25. Which is the URL escape code
for %.
Signed-off-by: Måns Ansgariusson [email protected]