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

check the size of seqs before trying to decode them #42

Merged
merged 1 commit into from May 28, 2015

Conversation

@jmesmon
Copy link
Contributor

jmesmon commented May 28, 2015

Fixes #41

Not sure if this style OOM issue affects other parts of the decoder.

@jmesmon
Copy link
Contributor Author

jmesmon commented May 28, 2015

It looks like at the least read_map() also needs help in this area.

@jmesmon
Copy link
Contributor Author

jmesmon commented May 28, 2015

It isn't clear to me that this is really the right solution: it presumes that size_of is the same size as the representation of T when bincoded (which isn't generally correct).

It is probably sufficient to assume that for this check, but I'm a bit concerned about potential corner cases where the size_of is significantly larger than bincoded T.

Perhaps the real solution here is to account separately for bytes read vs bytes allocated?

Maybe this is good enough for now?

@TyOverby
Copy link
Collaborator

TyOverby commented May 28, 2015

Oh wow. This is a pretty big issue; the kind that I had hoped to fix with SizeLimit in the first place.

I think this could be solved in a few other ways, but probably not as easily as this.

Because in-memory size is just as important as wire-size, I think I should rewrite the docs saying that it can fail with SizeLimit on either.

TyOverby added a commit that referenced this pull request May 28, 2015
check the size of seqs before trying to decode them
@TyOverby TyOverby merged commit 7fb561d into servo:master May 28, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@TyOverby
Copy link
Collaborator

TyOverby commented May 28, 2015

Fixing read_map is implemented in 628b989

TyOverby pushed a commit that referenced this pull request May 29, 2015
This reverts commit 7fb561d, reversing
changes made to 1e49e5a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.