-
Notifications
You must be signed in to change notification settings - Fork 593
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 for #1543 #1905
Fix for #1543 #1905
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, much simpler.
I believe you also need to remove the validation for the preserve-slice metadata (unless there is none?), but this can wait until a follow-up PR.
@@ -19,7 +18,7 @@ class ClientPermissionsVerifierI final : public Glacier2::PermissionsVerifier | |||
{ | |||
if(current.ctx.find("throw") != current.ctx.end()) | |||
{ | |||
throw Test::ExtendedPermissionDeniedException("reason"); | |||
throw Glacier2::PermissionDeniedException("reason"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding:
using a derived exception no longer works for this use-case because the intermediary (IceGrid) unmarshals and then remarshals the exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right.
cpp/include/Ice/Value.h
Outdated
|
||
private: | ||
|
||
::std::shared_ptr<Ice::SlicedData> _slicedData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: use SliceDataPtr like above:
::std::shared_ptr<Ice::SlicedData> _slicedData; | |
SlicedDataPtr _slicedData; |
I couldn't find metadata validation for preserve-slice. |
This PR fixes #1543 for all language mappins except Matlab. I can try to write a patch for Matlab to make it easier to fix it but I won't be able to test.