-
Notifications
You must be signed in to change notification settings - Fork 133
fix: trustless gateway returned blocks can be limited #791
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
Conversation
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.
self review
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.
LGTM, a few nits inline.
Co-authored-by: Alex Potsides <alex@achingbrain.net>
Co-authored-by: Alex Potsides <alex@achingbrain.net>
Co-authored-by: Alex Potsides <alex@achingbrain.net>
Co-authored-by: Alex Potsides <alex@achingbrain.net>
Co-authored-by: Alex Potsides <alex@achingbrain.net>
use lowercase for logs; sentence case for error messages
FYI all of the requested changes have been made. lmk if I missed something. I'll look over all the code again now |
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.
self review
packages/block-brokers/src/trustless-gateway/trustless-gateway.ts
Outdated
Show resolved
Hide resolved
* If the response contains a content-length header greater than the limit or the actual bytes returned are greater than | ||
* the limit, an error is thrown. | ||
*/ | ||
export async function limitedResponse (response: Response, byteLimit: number, options?: LimitedResponseOptions): Promise<Uint8Array> { |
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.
I left the property name here as byteLimit
but could change to maxSize
if we want.. leaving for now
packages/block-brokers/src/trustless-gateway/trustless-gateway.ts
Outdated
Show resolved
Hide resolved
* origin/main: (27 commits) fix: flaky test to publish an IPNS record (#810) docs: fix API docs link (#809) fix: trustless gateway returned blocks can be limited (#791) deps: bump kubo from 0.34.1 to 0.35.0 in the kubo-deps group (#806) chore: fix linting chore: release main (#805) chore: bump execa from 8.0.1 to 9.5.3 (#797) chore: bump @helia/unixfs from 4.0.3 to 5.0.2 in /benchmarks/add-dir (#796) chore: bump codecov/codecov-action from 5.4.0 to 5.4.3 (#802) chore: bump tinybench from 3.1.1 to 4.0.1 (#798) deps: update aegir to 47.x.x (#804) chore: dep groups again chore: restore groups chore: remove groups chore: release main (#793) deps: update all deps (#792) chore: release main (#774) feat: pass initial providers to session (#777) ci: uci/copy-templates (#782) fix: propagate ipns failures (#789) ...
Title
fix: trustless gateway returned blocks can be limited
Description
Adds a new utility function
limitedResponse
that ensures the response body is less than a given byte limit.This is done by:
content-length
header is less than the limit, if not, an error is thrown.Fixes #790
Notes & open questions
Change checklist