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 report) TypeError: Cannot read properties of undefined (reading 'attachmentId') when copy/pasting images #4566

Closed
ghgr opened this issue Jan 7, 2024 · 3 comments

Comments

@ghgr
Copy link

ghgr commented Jan 7, 2024

Trilium Version

0.62.4

What operating system are you using?

Ubuntu

What is your setup?

Server access only

Operating System Version

(Server) Ubuntu 22.04 LTS (Client) Debian 11

Description

When an image is copy pasted from Chromium into a new note, I get in the backend logs:

Saving image 'googlelogo_light_color_272x92dp.png' as attachment into note 'XNlX0j0UxZ76'
ERROR: Download of 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_light_color_272x92dp.png' for note 'XNlX0j0UxZ76' failed with error: Cannot read properties of undefined (reading 'attachmentId') TypeError: Cannot read properties of undefined (reading 'attachmentId')
    at downloadImage (/usr/src/app/src/services/notes.js:499:62)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Promise.all (index 0)

In /usr/src/app/src/services/notes.js:499 I see that attachment is undefined, but the function that returns it (imageService.saveImageToAttachment) is returning it correctly. If In notes.js I make attachment a variable instead of a const, it works.

Download of 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_light_color_272x92dp.png' succeeded and was saved as image attachment 'NZUGhkJ0sTay' of note 'XNlX0j0UxZ76'

But when I refresh the note it doesn't find the image anymore. It turns out that notes.js:514 is not writing the path to the local image correctly. I had to modify it to be like this:

    return content.replace(new RegExp(`\\s+src=[\"']${quotedUrl}[\"']`, "ig"), ` src="api/attachments/${encodeURIComponent(attachment.attachmentId)}/image/a"`);

And now it works. It downloads images pasted imaged and can display them properly. Unless the image url contains special symbols, like &. In that case, I get:

ERROR: Download of 'https://media.licdn.com/dms/image/D5603AQHv6LsdiUg1kw/profile-displayphoto-shrink_200_200/0/1695167344576?e=1710374400&v=beta&t=7_vu11I0nRPAA4SR2hJcQx9Ml6omXJMhKEpL0WeDDkY' for note 'XNlX0j0UxZ76' failed with error: Request to GET https://media.licdn.com/dms/image/D5603AQHv6LsdiUg1kw/profile-displayphoto-shrink_200_200/0/1695167344576?e=1710374400&v=beta&t=7_vu11I0nRPAA4SR2hJcQx9Ml6omXJMhKEpL0WeDDkY failed, error: 403 Forbidden Error: Request to GET https://media.licdn.com/dms/image/D5603AQHv6LsdiUg1kw/profile-displayphoto-shrink_200_200/0/1695167344576?e=1710374400&v=beta&t=7_vu11I0nRPAA4SR2hJcQx9Ml6omXJMhKEpL0WeDDkY failed, error: 403 Forbidden
    at generateError (/usr/src/app/src/services/request.js:202:12)
    at ClientRequest.<anonymous> (/usr/src/app/src/services/request.js:146:28)
    at ClientRequest.emit (node:events:517:28)
    at HTTPParser.parserOnIncomingClient (node:_http_client:700:27)
    at HTTPParser.parserOnHeadersComplete (node:_http_common:119:17)
    at TLSSocket.socketOnData (node:_http_client:541:22)
    at TLSSocket.emit (node:events:517:28)
    at addChunk (node:internal/streams/readable:335:12)
    at readableAddChunk (node:internal/streams/readable:308:9)
    at Readable.push (node:internal/streams/readable:245:10)

Don't be fooled by the Forbidden. It's not behind a login, but look at the "&". It turns out the url is not being correctly parsed by request.js. If I patch it like this (Line 116):

url: imageUrl.replace(/&amp;/g, '&')

It works now.

In Summary, I think I found 3 bugs:

  • notes.js Line 497. Attachment was not being stored as const. Replaced to var.
  • notes.js Line 514. Url to local image was not correctly set.
  • request.js Line 116. url was not being correctly escaped when containing &.

Error logs

No response

@ghgr ghgr added the Type: Bug label Jan 7, 2024
zadam added a commit that referenced this issue Jan 7, 2024
zadam added a commit that referenced this issue Jan 7, 2024
@zadam
Copy link
Owner

zadam commented Jan 7, 2024

Hello, thank you for the detailed report, should be fixed in the next patch release.

@zadam zadam closed this as completed Jan 7, 2024
@ghgr
Copy link
Author

ghgr commented Jan 8, 2024

Thank you @zadam !

Just a quick heads up, since you're now escaping the url, the function replaceUrl in notes.js:514 is not matching the original url, and therefore cannot replace the original link with the local one (at api/attachments/...). That means that the image is downloaded, but not linked in the note.

I tried replacing quotedUrl with url but still doesn't work.

In order to replicate:

  • Create a new note
  • Go to this linkedin profile (doesn't matter which one, let's take Bill Gates as an example): https://www.linkedin.com/in/williamhgates/
  • Right click on his profile picture and "Copy Image"
  • Paste into Trilium
  • The image shows
  • Go to "Note Source"
  • See that's pointing to the original source, not the local one api/attachments/...

You can see in the developer console that's making requests to the 3rd party site every time you open the note.

@ghgr ghgr mentioned this issue Jan 8, 2024
@zadam
Copy link
Owner

zadam commented Jan 9, 2024

@ghgr hello, thank you for further testing. The previous fix was unescaping the URL in an incorrect place which then made it impossible to find the correct URL in HTML to replace. Bill Gates is now getting properly replaced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants