Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix private file signed url generation #596

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

worgho2
Copy link
Contributor

@worgho2 worgho2 commented Dec 14, 2024

Description

When trying to display a PDF file, I've figured out that notion is using a new bucket url to serve their files and it impacts how this library handles url signing.

  • Old files start with: https://s3-us-west-2.amazonaws.com/secure.notion-static.com/...
  • New files start with: https://prod-files-secure.s3.us-west-2.amazonaws.com/...

Including prod-files-secure prefix when identifying a private url, fixed the issue.

Other than that, the PDF component is using an outdated pdfjs-dist cdn url, so i've updated it.

Notion Test Page ID

  • PDFs not being displayed: 34d650c65da34f888335dbd3ddd141dc

Copy link

vercel bot commented Dec 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
react-notion-x ✅ Ready (Inspect) Visit Preview Dec 14, 2024 5:03am
react-notion-x-minimal-demo ✅ Ready (Inspect) Visit Preview Dec 14, 2024 5:03am

@scriptify
Copy link

Good stuff, thanks

@transitive-bullshit transitive-bullshit merged commit 4985e0e into NotionX:master Jan 10, 2025
2 of 4 checks passed
@transitive-bullshit
Copy link
Member

Thanks @worgho2 🙏

@mfts
Copy link
Contributor

mfts commented Jan 12, 2025

@transitive-bullshit this is not yet published a new version, right?

@transitive-bullshit
Copy link
Member

@transitive-bullshit this is not yet published a new version, right?

Correct; will publish soon.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants