[
https://issues.apache.org/jira/browse/THRIFT-4562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16462024#comment-16462024
]
ASF GitHub Bot commented on THRIFT-4562:
----------------------------------------
Jens-G closed pull request #1551: THRIFT-4562 Calling wrong exception CTOR
leads to "call failed: unkno…
URL: https://github.com/apache/thrift/pull/1551
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc
b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc
index db06827602..8bd77e8427 100644
--- a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc
@@ -131,7 +131,7 @@ class t_delphi_generator : public t_oop_generator {
bool is_xception_class,
bool is_union,
bool is_xception_factory,
- std::string xception_factroy_name);
+ std::string xception_factory_name);
void generate_delphi_clear_union_value(ostream& out,
std::string cls_prefix,
std::string name,
@@ -141,7 +141,7 @@ class t_delphi_generator : public t_oop_generator {
bool is_xception_class,
bool is_union,
bool is_xception_factory,
- std::string xception_factroy_name);
+ std::string xception_factory_name);
void generate_delphi_isset_reader_impl(ostream& out,
std::string cls_prefix,
std::string name,
@@ -1483,8 +1483,6 @@ void
t_delphi_generator::generate_delphi_struct_impl(ostream& out,
indent_up_impl();
if (is_exception && (!is_x_factory)) {
indent_impl(out) << "inherited Create('');" << endl;
- indent_impl(out) << "F" << exception_factory_name << " := T" <<
exception_factory_name
- << "Impl.Create;" << endl;
} else {
indent_impl(out) << "inherited;" << endl;
}
@@ -1530,6 +1528,19 @@ void
t_delphi_generator::generate_delphi_struct_impl(ostream& out,
indent_down_impl();
indent_impl(out) << "end;" << endl << endl;
+ if (is_exception && (!is_x_factory)) {
+ indent_impl(out) << "function " << cls_prefix << cls_nm << "." <<
exception_factory_name
+ << ": I" << exception_factory_name << ";" << endl;
+ indent_impl(out) << "begin" << endl;
+ indent_up_impl();
+ indent_impl(out) << "if F" << exception_factory_name << " = nil" << endl;
+ indent_impl(out) << "then F" << exception_factory_name << " := T" <<
exception_factory_name << "Impl.Create;" << endl;
+ indent_impl(out) << endl;
+ indent_impl(out) << "result := F" << exception_factory_name << ";" << endl;
+ indent_down_impl();
+ indent_impl(out) << "end;" << endl << endl;
+ }
+
if (tstruct->is_union()) {
indent_impl(out) << "procedure " << cls_prefix << cls_nm << "."
<< "ClearUnionValues;" << endl;
@@ -1813,8 +1824,7 @@ void
t_delphi_generator::generate_delphi_struct_definition(ostream& out,
if (is_exception && (!is_x_factory)) {
out << endl;
indent(out) << "// Exception Factory" << endl;
- indent(out) << "property " << exception_factory_name << ": " <<
struct_intf_name << " read F"
- << exception_factory_name << " write F" <<
exception_factory_name << ";" << endl;
+ indent(out) << "function " << exception_factory_name << ": " <<
struct_intf_name << ";" << endl;
}
if ((!is_exception) || is_x_factory) {
@@ -3450,7 +3460,7 @@ void
t_delphi_generator::generate_delphi_property_writer_impl(ostream& out,
bool
is_xception_class,
bool is_union,
bool
is_xception_factory,
- std::string
xception_factroy_name) {
+ std::string
xception_factory_name) {
(void)type;
t_type* ftype = tfield->get_type();
@@ -3471,7 +3481,7 @@ void
t_delphi_generator::generate_delphi_property_writer_impl(ostream& out,
indent_impl(out) << fieldPrefix << prop_name(tfield, is_xception_class) << "
:= Value;" << endl;
if (is_xception_class && (!is_xception_factory)) {
- indent_impl(out) << "F" << xception_factroy_name << "." <<
prop_name(tfield, is_xception_class)
+ indent_impl(out) << xception_factory_name << "." << prop_name(tfield,
is_xception_class)
<< " := Value;" << endl;
}
@@ -3538,7 +3548,7 @@ void
t_delphi_generator::generate_delphi_create_exception_impl(ostream& out,
indent_impl(out) << "Result := " << exception_cls_nm << ".Create;" << endl;
string factory_name = normalize_clsnm(tstruct->get_name(), "", true) +
"Factory";
- indent_impl(out) << "Result." << factory_name << " := Self;" << endl;
+ indent_impl(out) << "Result.F" << factory_name << " := Self;" << endl;
const vector<t_field*>& fields = tstruct->get_members();
vector<t_field*>::const_iterator f_iter;
@@ -3548,8 +3558,7 @@ void
t_delphi_generator::generate_delphi_create_exception_impl(ostream& out,
for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
propname = prop_name(*f_iter, is_exception);
if ((*f_iter)->get_req() != t_field::T_REQUIRED) {
- indent_impl(out) << "if __isset_" << propname << " then" << endl;
- indent_impl(out) << "begin" << endl;
+ indent_impl(out) << "if __isset_" << propname << " then begin" << endl;
indent_up_impl();
}
indent_impl(out) << "Result." << propname << " := " << propname << ";" <<
endl;
diff --git a/lib/delphi/test/TestServer.pas b/lib/delphi/test/TestServer.pas
index 4400c342d1..69cb17521d 100644
--- a/lib/delphi/test/TestServer.pas
+++ b/lib/delphi/test/TestServer.pas
@@ -164,7 +164,7 @@ procedure TTestServer.TTestHandlerImpl.testException(const
arg: string);
if (arg = 'TException') then
begin
- raise TException.Create('');
+ raise TException.Create('TException');
end;
// else do not throw anything
diff --git a/lib/delphi/test/serializer/TestSerializer.dpr
b/lib/delphi/test/serializer/TestSerializer.dpr
index 4360a73d4d..51e22a4cf9 100644
--- a/lib/delphi/test/serializer/TestSerializer.dpr
+++ b/lib/delphi/test/serializer/TestSerializer.dpr
@@ -35,7 +35,7 @@ uses
Thrift.Serializer in '..\..\src\Thrift.Serializer.pas',
Thrift.Stream in '..\..\src\Thrift.Stream.pas',
Thrift.TypeRegistry in '..\..\src\Thrift.TypeRegistry.pas',
- ReservedKeywords,
+ System_,
DebugProtoTest,
TestSerializer.Data;
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Calling wrong exception CTOR leads to "call failed: unknown result" instead
> of the real exception being thrown
> --------------------------------------------------------------------------------------------------------------
>
> Key: THRIFT-4562
> URL: https://issues.apache.org/jira/browse/THRIFT-4562
> Project: Thrift
> Issue Type: Bug
> Components: Delphi - Compiler
> Reporter: Jens Geyer
> Priority: Major
>
> h2. Problem
> # In contrast to other languages, there is no real way to prevent Delphi from
> inheriting unwanted CTORs.
> # The generated code initializes an "exception factory" helper object used
> for serializing the exception info over the wire. This helper object is
> instantiated in two cases:
> ** a) in the CTORs that are defined in the exception class
> ** b) when a property setter is called on that instance
> h2. Example
> The following IDL:
> {code:java}
> exception ApiException {
> 1: string Msg
> 2: i32 Error
> }
> {code}
> results in this Delphi code:
> {code:java}
> TApiException = class(TException)
> // ... some unrelevant details omitted ...
> public
> constructor Create; overload;
> constructor Create( const AMsg: System.string; AError: System.Integer);
> overload;
> // ... more code ...
> end;
> {code}
> Due to the inheritance from {{TException}} and finally {{System.Exception}},
> a bunch of other CTORs is inherited, such as:
> {code:java}
> constructor Create(const Msg: string);
> {code}
> From this, a situation may arise, in which the developer instantiates the
> instance by calling one of the inherited CTORs (which do not intialize the
> Factory helper object), then raising the exception without assigning any
> other values via property setter:
> {code}
> raise TApiException.Create('some error occurred!'); // inherited CTOR
> {code}
> instead of
> {code}
> raise TApiException.Create('some error occurred!', 42); // TApiException
> CTOR
> {code}
> Under these circumstances, the Factory helper object is left at its default
> value of {{nil}}, hence no suitable information is written to the wire. As a
> direct result, being unable to deserialize anything useful on return, the
> client code will run into the default catcher code which raises an generic
> TApplicationException with {{MissingResult}} code and the "call failed:
> unknown result" message.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
