otris/ews-cpp

Read Access Violation on invalid response

Closed this issue · 5 comments

I used some bogus server URIs.
At some point, some server returned a very basic html response.
The parse function, in this case the sync_folder_items_result::parse(...),
used the get_element_by_qname(...) function to get the "SyncFolderItemResponseMessage" xml tag.
But since the response was bogus, the function returned a nullptr.
This nullptr is dereferenced to the parse_response_class_and_code(...) function and first_attribute is called on it, causing a Read Access Violation.

What is the intended way to handle this?
I expected an exception to be thrown, being able to catch it, handle it and go on.

This is the case for every parse function. If I'm not mistaken.

Yes, access violations (or at least SIGABRT via a failed assert()) are expected when the XML returned from the server is not as expected.

It seemed a good idea back when development of the library started because you'd see immediately where mistakes are being made in the parsing code.

In production, I think, this is not such a good idea to take a whole process down just because a remote server answers in an unexpected way.

This is the case for every parse function. If I'm not mistaken.

You're correct.

After a quick look at the sources, I think changing this behavior is not that difficult.

  1. I'd replace EWS_ASSERT to not use libc's assert() macro but to throw an actual C++ exception
  2. Make sure that EWS_ASSERT is always enabled, remove the ENABLE_ASSERTS option in CMakeLists.txt
  3. Add a test Python server that always answers with valid but unexpected XML and run the entire test suite against it, fixing every remaining issue
  4. Hope that the test suite has good coverage 🙏

Did I miss something?

57ede1d should solve items 1. and 2. of the list above.

Since PR #112 has been merged I like to close this issue. @SebastianBecker2 is this fine with you?

Works like a charm. Thanks a lot!