Skip to content

Commit 110fe88

Browse files
committed
Refactor ChangesetDiscussionBuilder to use offset instead of pointer for comment management to prevent corrupted memory access when the size of comments go larger and triggers buffer reallocation handling (e.g. changeset with id 62278154).
1 parent 6dd4e28 commit 110fe88

File tree

1 file changed

+43
-12
lines changed

1 file changed

+43
-12
lines changed

include/osmium/builder/osm_object_builder.hpp

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ namespace osmium {
324324

325325
class ChangesetDiscussionBuilder : public Builder {
326326

327-
osmium::ChangesetComment* m_comment = nullptr;
327+
std::size_t m_comment_offset = SIZE_MAX;
328328

329329
void add_user(osmium::ChangesetComment& comment, const char* user, const std::size_t length) {
330330
if (length > osmium::max_osm_string_length) {
@@ -343,6 +343,16 @@ namespace osmium {
343343
add_padding(true);
344344
}
345345

346+
// Get current comment pointer (recalculated each time to handle buffer reallocation)
347+
osmium::ChangesetComment* get_comment_ptr() {
348+
if (m_comment_offset == SIZE_MAX) {
349+
return nullptr;
350+
}
351+
return reinterpret_cast<osmium::ChangesetComment*>(
352+
buffer().data() + buffer().committed() + m_comment_offset
353+
);
354+
}
355+
346356
public:
347357

348358
explicit ChangesetDiscussionBuilder(osmium::memory::Buffer& buffer, Builder* parent = nullptr) :
@@ -362,32 +372,53 @@ namespace osmium {
362372
ChangesetDiscussionBuilder& operator=(ChangesetDiscussionBuilder&&) = delete;
363373

364374
~ChangesetDiscussionBuilder() {
365-
assert(!m_comment && "You have to always call both add_comment() and then add_comment_text() in that order for each comment!");
375+
assert(m_comment_offset == SIZE_MAX && "You have to always call both add_comment() and then add_comment_text() in that order for each comment!");
366376
add_padding();
367377
}
368378

369379
void add_comment(osmium::Timestamp date, osmium::user_id_type uid, const char* user) {
370-
assert(!m_comment && "You have to always call both add_comment() and then add_comment_text() in that order for each comment!");
371-
m_comment = reserve_space_for<osmium::ChangesetComment>();
372-
new (m_comment) osmium::ChangesetComment{date, uid};
380+
assert(m_comment_offset == SIZE_MAX && "You have to always call both add_comment() and then add_comment_text() in that order for each comment!");
381+
382+
// Store offset instead of pointer to handle buffer reallocation
383+
m_comment_offset = buffer().written() - buffer().committed();
384+
385+
auto* comment = reserve_space_for<osmium::ChangesetComment>();
386+
new (comment) osmium::ChangesetComment{date, uid};
373387
add_size(sizeof(ChangesetComment));
374-
add_user(*m_comment, user, std::strlen(user));
388+
389+
auto* comment_ptr = get_comment_ptr();
390+
add_user(*comment_ptr, user, std::strlen(user));
375391
}
376392

377393
void add_comment_text(const char* text) {
378-
assert(m_comment && "You have to always call both add_comment() and then add_comment_text() in that order for each comment!");
379-
osmium::ChangesetComment& comment = *m_comment;
380-
m_comment = nullptr;
394+
assert(m_comment_offset != SIZE_MAX && "You have to always call both add_comment() and then add_comment_text() in that order for each comment!");
395+
396+
// Get fresh pointer each time to handle buffer reallocation
397+
auto* comment_ptr = get_comment_ptr();
398+
osmium::ChangesetComment& comment = *comment_ptr;
399+
400+
// Invalidate offset to ensure right adding order
401+
m_comment_offset = SIZE_MAX;
402+
381403
add_text(comment, text, std::strlen(text));
382404
}
383405

384406
void add_comment_text(const std::string& text) {
385-
assert(m_comment && "You have to always call both add_comment() and then add_comment_text() in that order for each comment!");
386-
osmium::ChangesetComment& comment = *m_comment;
387-
m_comment = nullptr;
407+
assert(m_comment_offset != SIZE_MAX && "You have to always call both add_comment() and then add_comment_text() in that order for each comment!");
408+
409+
// Get fresh pointer each time to handle buffer reallocation
410+
auto* comment_ptr = get_comment_ptr();
411+
osmium::ChangesetComment& comment = *comment_ptr;
412+
413+
// Invalidate offset to ensure right adding order
414+
m_comment_offset = SIZE_MAX;
415+
388416
add_text(comment, text.c_str(), text.size());
389417
}
390418

419+
bool has_open_comment() const noexcept {
420+
return m_comment_offset != SIZE_MAX;
421+
}
391422
}; // class ChangesetDiscussionBuilder
392423

393424
#define OSMIUM_FORWARD(setter) \

0 commit comments

Comments
 (0)