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

🐞 [Bug]: Channels URL for QR Codes Aren't Properly Formatted #752

Closed
1 task done
AddisonTustin opened this issue Jul 6, 2024 · 4 comments
Closed
1 task done
Labels
bug Something isn't working

Comments

@AddisonTustin
Copy link

AddisonTustin commented Jul 6, 2024

Firmware Version

latest

What did you do?

The syntax of a URI/URL requires that the fragment (the portion of the URI that follows a #) should be at the very end of the URI. When beginning to work on meshtastic/Meshtastic-Android#953 I noticed that since ?add=true followed the fragment, Java's native URI feature does not properly parse add=true as a query param, and instead includes it as part of the fragment.

There's some cleanup I'd like to do as a part of meshtastic/Meshtastic-Android#953 to use the URI directly rather than doing manual string processing. So this becomes a small blocker.

Expected Behavior

The query params (?add=true) should be included in the URI before the fragment.

Link to URI Generic Syntax (Library of Congress).

Current Behavior

When constructing the channelsUrl ?add=true is appended to the end of the URI.

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

I'm willing to submit a PR, but I don't currently have a dev environment setup for iOS application development. So if there are others willing to tackle this before I have an opportunity to, that would be appreciated 🙂

@AddisonTustin AddisonTustin added the bug Something isn't working label Jul 6, 2024
@garthvh
Copy link
Member

garthvh commented Jul 6, 2024

It is after the hash so it does not get sent to the server. React uses this order as well.

@AddisonTustin
Copy link
Author

AddisonTustin commented Jul 6, 2024

The actual channel configurations are in the fragment to reduce the chance of leaking channel keys, right? Not sure if it's as meaningful a distinction for the query string. Though that may not be a super compelling reason, so happy to close this issue and just process the QR code link appropriately in the Android client if that's the preference here.

@garthvh
Copy link
Member

garthvh commented Jul 8, 2024

I moved it before the hash, I can handle both easily enough, probably a pain in js too

@garthvh
Copy link
Member

garthvh commented Jul 10, 2024

This is in app review

@garthvh garthvh closed this as completed Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants