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

Fix escaping bug #14

Merged
merged 16 commits into from
Oct 9, 2015
Merged

Fix escaping bug #14

merged 16 commits into from
Oct 9, 2015

Conversation

ppikula
Copy link

@ppikula ppikula commented Sep 15, 2015

see #11 for details

@michalwski
Copy link

Should we try MongooseIM and/or escalus with this exml version before merge?

@ppikula
Copy link
Author

ppikula commented Sep 16, 2015

Yes, I am going to do it in two steps:

  1. use this branch in Mongoose only and see what breaks
  2. use this branch in Mongoose and in tests+escalus

@ppikula ppikula changed the title Fix escaping bug [WIP] Fix escaping bug Sep 16, 2015
Pawel Pikula added 3 commits September 16, 2015 14:10
If we keep the types inside modules(not in hrl) we can shorten their names, instead
of exml_stream:xmlstreamstart() we can write exml_stream:start(), exml:cdata() etc.
btw I renamed stream end to stream stop, reasoning it should be either start/stop
or begin end.
@ppikula ppikula changed the title [WIP] Fix escaping bug Fix escaping bug Oct 2, 2015
@ppikula
Copy link
Author

ppikula commented Oct 8, 2015

This branch was battle(load) tested during previous 2 weeks, both in Mongoose and tests.

Summary:

  • xml_size function was added - calculates size of xmlel record
  • we are escaping every attribute & cdata - this allow a programmer to not think about it an possibly forget about doing this in crucial places.
  • switched binary allocator: enif_alloc_binary -> enif_make_new_binary, because the first one always allocates refcounted binary, that leads to increased memory usage in case of small binaries.
  • after changing the allocators buffer overflow revealed. It was one byte and was able to cause seg fault of the whole VM see: df539c8
  • I changed types, IMO they are more readable and consistent.
  • Improved coverage

michalwski added a commit that referenced this pull request Oct 9, 2015
* xml_size function was added  - calculates size of xmlel record 
* we are escaping every attribute & cdata - this allow a programmer to not think about it an possibly forget about doing this in crucial places.
* switched binary allocator: `enif_alloc_binary` -> `enif_make_new_binary`, because the first one **always** allocates refcounted binary, that leads to increased memory usage in case of small binaries. 
* after changing the allocators buffer overflow revealed. It was one byte and was able to cause seg fault of the whole VM see: df539c8
* I changed types, IMO they are more readable and consistent.
* Improved coverage
@michalwski michalwski merged commit d7b745f into master Oct 9, 2015
@michalwski michalwski deleted the fix-escaping-bug branch December 1, 2015 10:17
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.

5 participants