-
Notifications
You must be signed in to change notification settings - Fork 31
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
feat: add ReadFileWithSizeLimit
and use it on x/wasm
#696
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #696 +/- ##
==========================================
+ Coverage 61.18% 61.23% +0.05%
==========================================
Files 873 874 +1
Lines 98501 98546 +45
==========================================
+ Hits 60265 60349 +84
+ Misses 34667 34619 -48
- Partials 3569 3578 +9
|
ReadFileWithSizeLimit
and use it on x/wasm
ReadFileWithSizeLimit
and use it on x/wasm
ReadFileWithSizeLimit
and use it on x/wasm
ReadFileWithSizeLimit
and use it on x/wasm
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
Add to address it on x/distribution |
CHANGELOG.md
Outdated
* (x/wasm) [\#696](https://github.com/line/lbm-sdk/pull/696) x/wasm - add checking a wasm file size before reading it | ||
* (x/distribution) [\#696](https://github.com/line/lbm-sdk/pull/696) x/distribution - add checking a proposal file size before reading it |
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.
How about changing one line?
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've updated it. Thanks
Description
Reading a huge file has the risk of the crash application by using
ioutil.ReadFile/os.ReadFile
.Here is the updates:
ReadFileWithSizeLimit
Motivation and context
How has this been tested?
Screenshots (if appropriate):
Checklist:
CHANGELOG.md
client/docs/swagger-ui/swagger.yaml