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.

Sign up for GitHub

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 large heap allocation during load broken files. #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix large heap allocation during load broken files. #46

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions kaitai/kaitaistream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,19 +379,20 @@ uint64_t kaitai::kstream::get_mask_ones(int n) {
// ========================================================================

std::string kaitai::kstream::read_bytes(std::streamsize len) {
std::vector<char> result(len);

// NOTE: streamsize type is signed, negative values are only *supposed* to not be used.
// http://en.cppreference.com/w/cpp/io/streamsize
if (len < 0) {
throw std::runtime_error("read_bytes: requested a negative amount");
} else if (len == 0) {
return std::string();
} else if (len > size()) {
throw std::runtime_error("read_bytes: requested length greater than stream size");
}
Copy link
Member

Choose a reason for hiding this comment

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

In general, that's a bad idea to call size() if we can avoid that. First of all, not all streams are finite and have sizes at all. The way that size() is implemented in KS runtime is by seeking to end of stream, getting position index there, and going back to pre-memorized original location. It won't work on infinite streams like ones coming from IO port or networking socket.

Moreover, reading past end-of-stream throws a well-defined exception like std::ifstream::failure, is (1) arguably better than throwing a std::runtime_error with some text, (2) relied upon in existing codebases.

Copy link
Author

Choose a reason for hiding this comment

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

As I understand kaitaia::ostream can not work with infinite/non seekable streams. See the realization of kaitai::kstream::read_bytes_term(...), kaitai::kstream::size(...), kaitai::kstream::seek(...).


if (len > 0) {
m_io->read(&result[0], len);
}
std::string result(len, ' ');
m_io->read(&result[0], len);

return std::string(result.begin(), result.end());
return result;
}

std::string kaitai::kstream::read_bytes_full() {
Expand Down