Skip to content

fix: Fixes importing url encoded recipe deeplinks #3446

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

Merged
angiejones merged 1 commit into from
Jul 17, 2025
Merged

fix: Fixes importing url encoded recipe deeplinks #3446

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ui/desktop/src/components/RecipesView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ export default function RecipesView({ onBack }: RecipesViewProps) {
if (!configBase64) {
throw new Error('No recipe configuration found in deeplink');
}
const configJson = Buffer.from(configBase64, 'base64').toString('utf-8');
const urlDecoded = decodeURIComponent(configBase64);
Copy link
Collaborator

@DOsinga DOsinga Jul 16, 2025

Choose a reason for hiding this comment

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

I think if the configBase64 was not url encoded and it does contain a +, this will cause issues

Copy link
Contributor Author

@jsibbison-square jsibbison-square Jul 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah I think we should officially document the format of recipe deeplinks so that external integrations know how to use them. I expect we should use the url_safe version and then url-encode. Unless we are going the route of url_safe_no_pad.

These changes are backwards incompatible which is why I'm hoping we can make an official/decision statement so we have a position to operate from.

Copy link
Collaborator

@DOsinga DOsinga Jul 17, 2025

Choose a reason for hiding this comment

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

They are backwards compatible, but since we have both used url-encoded and not url-encoded versions, it doesn't quite fix it.

is there a point in url-encoding the url_safe version?

Copy link
Contributor Author

@jsibbison-square jsibbison-square Jul 17, 2025

Choose a reason for hiding this comment

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

Seems like even with base64::engine::general_purpose::URL_SAFE we still want to encode it for maximum interoperability. Eg. %3D unencoded is = which would break if this was part of a query url or form submit.

https://docs.rs/base64/latest/base64/#url-safe-alphabet

Copy link
Collaborator

@DOsinga DOsinga Jul 17, 2025

Choose a reason for hiding this comment

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

I am not sure I follow - the whole point of url safe base64 is that you can use it in, eh, urls. so url safe base64 with the = stripped seems like the most compact and portable way that we can do this

for decoding we'd have to put a bunch of safety measures in place probably, so still try to url-decode it if the base64 decode doesn't work etc

Copy link
Contributor Author

@jsibbison-square jsibbison-square Jul 17, 2025

Choose a reason for hiding this comment

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

👍 yep we can use URL_SAFE_NO_PAD so the = are never added into the strings (so no need to url encode). It just means that standard base64 decoding won't work unless you add the NO_PAD option. https://docs.rs/base64/latest/base64/#padding-characters

We will still have to have backwards compatible decoding for the old ones we generated, so ultimately won't simplify any code but atleast there will be a standard going forward.

Copy link
Contributor Author

@jsibbison-square jsibbison-square Jul 17, 2025

Choose a reason for hiding this comment

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

I'm working on having the encoding/decoding only done on the server.

Copy link
Collaborator

@DOsinga DOsinga Jul 17, 2025

Choose a reason for hiding this comment

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

great - do you want to do that in this, or merge this first

Copy link
Contributor Author

@jsibbison-square jsibbison-square Jul 18, 2025

Choose a reason for hiding this comment

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

Implemented: #3489

const configJson = Buffer.from(urlDecoded, 'base64').toString('utf-8');
const recipe = JSON.parse(configJson) as Recipe;

if (!recipe.title || !recipe.description || !recipe.instructions) {
Expand Down
Loading