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

Add zlib inflated data size limit #167

Merged
merged 3 commits into from
Apr 9, 2014
Merged

Add zlib inflated data size limit #167

merged 3 commits into from
Apr 9, 2014

Conversation

fenek
Copy link
Member

@fenek fenek commented Apr 7, 2014

Without such limit server can crash or use lots of resources when inflating data with very high compression ratio.

<<0, In/binary>> ->
{ok, In};
<<1, Error/binary>> ->
{error, binary_to_list(Error)}
{error, erlang:binary_to_existing_atom(Error, latin1)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please, use ut8 here. I know that both latin1 and utf8 will work here, but in other places MongooseIM is using utf8 encoding (for example xml parser) so it's better to be coherent in all places.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do. :)

@fenek
Copy link
Member Author

fenek commented Apr 9, 2014

Is everything fine now? :)

@@ -221,7 +221,10 @@ init([{SockMod, Socket}, Opts]) ->
{value, {_, XS}} -> XS;
_ -> false
end,
Zlib = lists:member(zlib, Opts),
Zlib = case lists:keysearch(zlib, 1, Opts) of
Copy link
Member

Choose a reason for hiding this comment

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

From man 3 lists:

This function is retained for backward compatibility.
The function lists:keyfind/3 (introduced in R13A) is in most cases more convenient.

Copy link
Member Author

Choose a reason for hiding this comment

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

keysearch is used in this block of code multiple times, I wanted it to be consistent. Should we replace all keysearch occurences in the code then? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please :)

@fenek
Copy link
Member Author

fenek commented Apr 9, 2014

And now? :P

Also replace keysearch with keyfind in c2s
@fenek
Copy link
Member Author

fenek commented Apr 9, 2014

Squashed formatting commits, should be ready for merge.

michalwski added a commit that referenced this pull request Apr 9, 2014
Add zlib inflated data size limit
@michalwski michalwski merged commit 586d96c into master Apr 9, 2014
@fenek fenek deleted the zlib_cwe400_fix branch April 10, 2014 08:44
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