Skip to content

Commit 8e4dd61

Browse files
committed
Fix problem with storing reference to a temporary object.
This could happen because of implicit conversion from mysqlx string to cdk string when passing id parameter from add_or_replace_one() to Value_expr_check_id ctor.
1 parent bec9e93 commit 8e4dd61

File tree

1 file changed

+52
-5
lines changed

1 file changed

+52
-5
lines changed

devapi/crud.cc

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,15 @@ struct Upsert_cmd : public Executable<Result, Upsert_cmd>
146146
};
147147

148148

149+
/*
150+
A helper class used by Collection_detail::add_or_replace_one().
151+
152+
It is a wrapper around CDK expression m_expr that describes a document.
153+
The wrapper forwards this description to a processor, but at the same
154+
time checks if the value of the (top-level) "_id" field equals the value
155+
given in the constructor.
156+
*/
157+
149158
struct Value_expr_check_id
150159
: cdk::Expression
151160
, cdk::Expression::Processor
@@ -157,6 +166,19 @@ struct Value_expr_check_id
157166
Processor *m_prc;
158167
Doc_prc *m_doc_prc;
159168

169+
/*
170+
This class defines m_any_prc member which is used below
171+
to check the value of "_id" field as reported by the
172+
source expression. Before using this class, m_id_prc must be
173+
set to point at the sub-processor that was given for processing
174+
the value of the "_id" field (this is done by key_val() callback in
175+
the main class).
176+
177+
Then all callbacks are forwarded to this sub-processor (or its
178+
sub-processors) and in case of calling scalar str() callback that
179+
gives string value of the "_id" field, the check is done first.
180+
*/
181+
160182
struct Any_processor_check
161183
: cdk::Expression::Processor::Doc_prc::Any_prc
162184
, cdk::Expression::Processor::Doc_prc::Any_prc::Scalar_prc
@@ -165,9 +187,9 @@ struct Value_expr_check_id
165187
Any_prc *m_id_prc;
166188
Scalar_prc *m_scalar_prc;
167189
Value_prc *m_value_prc;
168-
const string& m_id;
190+
const std::string &m_id;
169191

170-
Any_processor_check(const string& id)
192+
Any_processor_check(const std::string& id)
171193
: m_id(id)
172194
{}
173195

@@ -231,6 +253,7 @@ struct Value_expr_check_id
231253
}
232254

233255
// Value processor implementation
256+
234257
void null() override { m_value_prc->null();}
235258

236259
void value(cdk::Type_info type,
@@ -246,6 +269,7 @@ struct Value_expr_check_id
246269
throw mysqlx::Error(R"(Document "_id" and replace id are different!)");
247270
m_value_prc->str(val);
248271
}
272+
249273
void num(int64_t val) override { m_value_prc->num(val); }
250274
void num(uint64_t val) override { m_value_prc->num(val); }
251275
void num(float val) override { m_value_prc->num(val); }
@@ -256,7 +280,7 @@ struct Value_expr_check_id
256280

257281
Any_processor_check m_any_prc;
258282

259-
Value_expr_check_id(mysqlx::Value_expr &expr, bool is_expr, const string& id)
283+
Value_expr_check_id(mysqlx::Value_expr &expr, bool is_expr, const std::string& id)
260284
: m_expr(expr)
261285
, m_is_expr(is_expr)
262286
, m_any_prc(id)
@@ -324,7 +348,14 @@ Collection_detail::add_or_replace_one(
324348
const mysqlx::string &id, mysqlx::Value &&doc, bool replace
325349
)
326350
{
351+
/*
352+
This is implemented by executing Replace_cmd or Upsert_command
353+
which internally use Op_collection_replace or Op_collection_upsert
354+
to perform relevant operation on the server.
355+
*/
356+
327357
Object_ref coll(get_schema().m_name, m_name);
358+
std::string id_str(id);
328359

329360

330361
if (!Value::Access::is_expr(doc) &&
@@ -333,13 +364,29 @@ Collection_detail::add_or_replace_one(
333364
doc = DbDoc(doc.get<string>());
334365
}
335366

367+
/*
368+
expr is a CDK expression object which describes the document
369+
to be added.
370+
*/
371+
336372
Value_expr expr(doc, parser::Parser_mode::DOCUMENT);
337373

338374
if (replace)
339375
{
340-
Value_expr_check_id check_id(expr, Value::Access::is_expr(doc), id);
376+
/*
377+
Replace_cmd executes Op_collection_replace which picks a document
378+
with the given id and replaes it with the document given as the last
379+
argument.
380+
381+
The document expression is wrapped in Value_expr_check_id to check
382+
if the "_id" field (if present) stores the correct document id
383+
and throws error if it is not the case (otherwise Replace_cmd
384+
would modify the "_id" field to match the given id).
385+
*/
386+
387+
Value_expr_check_id check_id(expr, Value::Access::is_expr(doc), id_str);
341388

342-
Replace_cmd cmd(m_sess, coll, std::string(id), check_id);
389+
Replace_cmd cmd(m_sess, coll, id_str, check_id);
343390
return cmd.execute();
344391
}
345392
else

0 commit comments

Comments
 (0)