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

fix: api permacache validate content length header #98

Merged

Conversation

vasco-santos
Copy link
Contributor

@vasco-santos vasco-santos commented May 19, 2022

This PR Validates content length exists on response from gateway, given it is required by R2. If there is no content length, buffers it in memory and adds to R2 the new Response to get the content length

Related to https://github.com/protocol/bifrost-infra/issues/1868

TODO:

  • Add limits to docs

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented May 19, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: e17f833
Status: ✅  Deploy successful!
Preview URL: https://a3328f0d.nftstorage-link.pages.dev

View logs

@vasco-santos vasco-santos force-pushed the fix/api-permacache-post-validate-content-length-header branch from b893863 to 37aa131 Compare May 19, 2022 11:26
Comment on lines +115 to +113
// Content length is mandatory given R2 needs a known size of the readable stream
// TODO: We need public gateways to set content-length for JSON content (maybe others)
// https://github.com/protocol/bifrost-infra/issues/1868
Copy link

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @lidel ❤️

@vasco-santos vasco-santos force-pushed the fix/api-permacache-post-validate-content-length-header branch from 37aa131 to c42e537 Compare May 20, 2022 14:42
@vasco-santos vasco-santos marked this pull request as ready for review May 20, 2022 16:28
@vasco-santos
Copy link
Contributor Author

vasco-santos commented May 20, 2022

We decided to move forward with this solution for MVP until we have a way of buffering content in external service or a fix to get content length here.

@vasco-santos vasco-santos merged commit e28d2c0 into main May 23, 2022
@vasco-santos vasco-santos deleted the fix/api-permacache-post-validate-content-length-header branch May 23, 2022 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants