Skip to content

Fix to use url-safe base64 in note url#743

Merged
SISheogorath merged 9 commits intomasterfrom
fix-to-use-url-safe-base64
Mar 18, 2018
Merged

Fix to use url-safe base64 in note url#743
SISheogorath merged 9 commits intomasterfrom
fix-to-use-url-safe-base64

Conversation

@jackycute
Copy link
Copy Markdown
Member

fix #742

Signed-off-by: Max Wu <jackymaxj@gmail.com>
@jackycute jackycute changed the title Remove and replace all note id compression in LZString with base64url Fix to use url-safe base64 in note url Feb 26, 2018
Copy link
Copy Markdown
Contributor

@SISheogorath SISheogorath left a comment

Choose a reason for hiding this comment

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

LGTM. Old links still work, new link style is generated.

Probably needs some further testing, but this can probably done in release.

@SISheogorath SISheogorath added bug Something isn't working refactor labels Feb 26, 2018
@SISheogorath SISheogorath added this to the 1.1.0-CE Release milestone Feb 26, 2018
Signed-off-by: Max Wu <jackymaxj@gmail.com>
@jackycute
Copy link
Copy Markdown
Member Author

@DenisValeev Please check the latest commit in this PR.

@DenisValeev
Copy link
Copy Markdown

Perfect

@jackycute jackycute force-pushed the fix-to-use-url-safe-base64 branch from c2f4c67 to d08c952 Compare March 3, 2018 08:26
Comment thread public/js/history.js Outdated
notehistory[i].id = encodeNoteId(id)
}
} catch (err) {
// na
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mhm, maybe at least some logging would be nice.

@jackycute jackycute force-pushed the fix-to-use-url-safe-base64 branch 2 times, most recently from 9cc724a to d08c952 Compare March 10, 2018 08:50
Signed-off-by: Max Wu <jackymaxj@gmail.com>
that LZString note url could be parsed by base64url note url and thus return wrong note id

Signed-off-by: Max Wu <jackymaxj@gmail.com>
Signed-off-by: Max Wu <jackymaxj@gmail.com>
@jackycute
Copy link
Copy Markdown
Member Author

5e975cb also fix #727

Signed-off-by: Max Wu <jackymaxj@gmail.com>
Signed-off-by: Max Wu <jackymaxj@gmail.com>
Copy link
Copy Markdown
Contributor

@SISheogorath SISheogorath left a comment

Choose a reason for hiding this comment

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

LGTM

@SISheogorath
Copy link
Copy Markdown
Contributor

@jackycute do you think it's safe to merge? Or does it need further testing?

@jackycute
Copy link
Copy Markdown
Member Author

It's good for now 👍

@SISheogorath SISheogorath merged commit f6df2de into master Mar 18, 2018
@SISheogorath
Copy link
Copy Markdown
Contributor

Thanks a lot for your work @jackycute!

@SISheogorath SISheogorath deleted the fix-to-use-url-safe-base64 branch March 18, 2018 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants