-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Desktop: Add OneNote Importer #10642
base: dev
Are you sure you want to change the base?
Conversation
…lib' into add-onenote-parser-lib
|
Since we talked in the last meeting about finding a way to remove the requirement of adding Rust to the docker image for the server what I did was use Inside the |
Dockerfile.server
Outdated
@@ -79,7 +84,7 @@ ARG BUILD_DATE | |||
ARG REVISION | |||
ARG VERSION | |||
LABEL org.opencontainers.image.created="$BUILD_DATE" \ | |||
org.opencontainers.image.title="Joplin Server" \ | |||
org.opencontainers.image.title="Joplin Server WITH RUST" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that debugging code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sorry, I'm going to clean this up
for (const encodedLink of fileLinks) { | ||
const link = decodeURI(encodedLink); | ||
|
||
if (isDataUrl(link)) { | ||
if (isDataUrl(link) || isMailTo(link) || isFilenameTooLong(link)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That fix should be moved to a separate PR
} | ||
|
||
public static setIdGenerator(generator: ()=> string) { | ||
this.uuidGenerator = generator; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A good pattern for this kind of setter is to return the previous generator so that it can be reset by the caller. That's more future proof.
packages/utils/html.ts
Outdated
parser.write(html); | ||
parser.end(); | ||
|
||
return { svgs, html: body.join('') }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the impact of this change on the app security? Looking at the code, I feel it will have the same SVG-related vulnerabilities that we already dealt with before. What are your thoughts on this? I guess it depends how the extracted SVGs are going to be used.
And as we've discussed before an HTML parser is not capable of parsing an SVG element safely. Maybe a different, more secure parser would be needed?
packages/utils/url.ts
Outdated
export const isFilenameTooLong = (filename: string) => { | ||
return filename.length > 255; | ||
}; | ||
|
||
export const isLink = (text: string) => { | ||
if (!text) return false; | ||
const linkRegex = /^(https?|file|joplin):\/\/[^)\s]+$/; | ||
return !!text.match(linkRegex); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do you need these? It's not really generic functions so I'm not sure we need it there
|
Summary
I'm adding a new package library that will be used to convert OneNote exports into Joplin Notes.
The onenote-converter is based on the implementations found in:
https:/msiemens/one2html
https:/msiemens/onenote.rs
We will be compiling the Rust code to wasm with the package
wasm-pack
to output a Node package that can be imported in our codebaseWhat is missing at this point:
onenote-converter
package (described bellow).run_ci.sh
(changed to debug)yarn dist
process (make sure the package is always build there)How the package is being integrated
Now Rust is a requirement, the message that is visible when Rust is not installed in the developer machine:
Error message
Message that shows when you try to run yarn build from the package without having Rust: