Skip to content
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

#610 cant send image in group chat #637

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ IncludeCategories:
- Regex: '^"config\.h"'
Priority: -1
# The main header for a source file automatically gets category 0
- Regex: '.*'
- Regex: '^<.*>'
Priority: 1
- Regex: '^<.*\.h>'
- Regex: '.*'
Priority: 2
4 changes: 2 additions & 2 deletions examples/count-bot/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ void processNewMessageEvent(shared_ptr<CoreThread> ct,
LOG(INFO) << "New Message Event: " << endl;
LOG(INFO) << " From: " << para.getPal()->GetKey().ToString() << endl;
for (auto& chip : para.dtlist) {
LOG(INFO) << " Message: " << chip.ToString() << endl;
LOG(INFO) << " Message: " << chip->ToString() << endl;
ostringstream oss;
oss << "your message has " << chip.data.size() << " bytes.";
oss << "your message has " << chip->data.size() << " bytes.";
ct->SendMessage(para.getPal(), oss.str());
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/api/iptux-core/CoreThread.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,17 @@ class CoreThread {
* @return false if send failed
*/
bool SendMessage(CPPalInfo pal, const std::string& message);
bool SendMessage(CPPalInfo pal, const ChipData& chipData);
bool SendMessage(CPPalInfo pal, std::shared_ptr<ChipData> chipData);
bool SendMsgPara(std::shared_ptr<MsgPara> msgPara);
void AsyncSendMsgPara(std::shared_ptr<MsgPara> msgPara);
void SendUnitMessage(const PalKey& palKey,
uint32_t opttype,
const std::string& message);
void SendUnitMessage(const PalKey& palKey,
uint32_t opttype,
std::shared_ptr<MsgPara> msgPara);
void SendGroupMessage(const PalKey& palKey, const std::string& message);
void SendGroupMessage(const PalKey& palKey, std::shared_ptr<MsgPara> msgPara);

bool SendAskShared(PPalInfo pal);
bool SendAskSharedWithPassword(const PalKey& palKey,
Expand Down
13 changes: 10 additions & 3 deletions src/api/iptux-core/Models.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,15 @@ using PFileInfo = std::shared_ptr<FileInfo>;
*/
class ChipData {
public:
static std::shared_ptr<ChipData> newTxtMsg(const std::string& text);
static std::shared_ptr<ChipData> newImgMsg(const std::string& picPath,
bool deleteFileAfterSent = true);

private:
explicit ChipData(const std::string& data);
ChipData(MessageContentType type, const std::string& data);

public:
~ChipData();

std::string ToString() const;
Expand Down Expand Up @@ -226,9 +233,9 @@ class MsgPara {

CPPalInfo getPal() const { return pal; }

MessageSourceType stype; ///< 来源类型
GroupBelongType btype; ///< 所属类型
std::vector<ChipData> dtlist; ///< 数据链表 *
MessageSourceType stype; ///< 来源类型
GroupBelongType btype; ///< 所属类型
std::vector<std::shared_ptr<ChipData>> dtlist; ///< 数据链表 *
private:
CPPalInfo pal; ///< 好友数据信息(来自好友*)
};
Expand Down
16 changes: 6 additions & 10 deletions src/iptux-core/CoreThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -477,12 +477,11 @@ bool CoreThread::SendMessage(CPPalInfo palInfo, const string& message) {
return true;
}

bool CoreThread::SendMessage(CPPalInfo pal, const ChipData& chipData) {
auto ptr = chipData.data.c_str();
switch (chipData.type) {
bool CoreThread::SendMessage(CPPalInfo pal, shared_ptr<ChipData> chipData) {
switch (chipData->type) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Null pointer check for chipData

Ensure that chipData is not null before dereferencing it to avoid potential null pointer dereference.

case MessageContentType::STRING:
/* 文本类型 */
return SendMessage(pal, chipData.data);
return SendMessage(pal, chipData->data);
case MESSAGE_CONTENT_TYPE_PICTURE:
/* 图片类型 */
int sock;
Expand All @@ -491,12 +490,9 @@ bool CoreThread::SendMessage(CPPalInfo pal, const ChipData& chipData) {
strerror(errno));
return false;
}
Command(*this).SendSublayer(sock, pal, IPTUX_MSGPICOPT, ptr);
Command(*this).SendSublayer(sock, pal, IPTUX_MSGPICOPT,
chipData->data.c_str());
Comment on lines +493 to +494
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Potential resource leak

Ensure that the socket is closed in case of an exception to avoid potential resource leaks.

close(sock); // 关闭网络套接口
/*/* 删除此图片 */
if (chipData.GetDeleteFileAfterSent()) {
unlink(ptr); // 此文件已无用处
}
return true;
default:
g_assert_not_reached();
Expand All @@ -506,7 +502,7 @@ bool CoreThread::SendMessage(CPPalInfo pal, const ChipData& chipData) {
bool CoreThread::SendMsgPara(shared_ptr<MsgPara> para) {
for (int i = 0; i < int(para->dtlist.size()); ++i) {
if (!SendMessage(para->getPal(), para->dtlist[i])) {
LOG_ERROR("send message failed: %s", para->dtlist[i].ToString().c_str());
LOG_ERROR("send message failed: %s", para->dtlist[i]->ToString().c_str());
return false;
}
}
Expand Down
9 changes: 4 additions & 5 deletions src/iptux-core/CoreThreadTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ TEST(CoreThread, SendMessage_ChipData) {
CoreThread* thread = new CoreThread(core);
auto pal = make_shared<PalInfo>("127.0.0.1", 2425);
thread->AttachPalToList(pal);
ChipData chipData("hello world");
auto chipData = ChipData::newTxtMsg("hello world");
EXPECT_TRUE(thread->SendMessage(pal, chipData));
delete thread;
}
Expand All @@ -95,7 +95,7 @@ TEST(CoreThread, SendMsgPara) {
CoreThread* thread = new CoreThread(core);
PPalInfo pal = make_shared<PalInfo>("127.0.0.1", 2425);
thread->AttachPalToList(pal);
ChipData chipData("hello world");
auto chipData = ChipData::newTxtMsg("hello world");
auto para = make_shared<MsgPara>(pal);
para->dtlist.push_back(std::move(chipData));
EXPECT_TRUE(thread->SendMsgPara(para));
Expand Down Expand Up @@ -179,7 +179,7 @@ TEST(CoreThread, FullCase) {
}

auto event2 = dynamic_pointer_cast<const NewMessageEvent>(event);
ASSERT_EQ(event2->getMsgPara().dtlist[0].ToString(),
ASSERT_EQ(event2->getMsgPara().dtlist[0]->ToString(),
"ChipData(MessageContentType::STRING, hello world)");
{
lock_guard<std::mutex> l(thread2EventsMutex);
Expand All @@ -204,8 +204,7 @@ TEST(CoreThread, FullCase) {
thread1->SendAskShared(pal2InThread1);

// send picture
ChipData chipData(MessageContentType::PICTURE, testDataPath("iptux.png"));
chipData.SetDeleteFileAfterSent(false);
auto chipData = ChipData::newImgMsg(testDataPath("iptux.png"), false);
thread1->SendMessage(pal2InThread1, chipData);
// WARNING: does not work as expected, the message will be sent from
// 127.0.0.2(expect 127.0.0.1) while(thread2Events.size() != 2) {
Expand Down
110 changes: 69 additions & 41 deletions src/iptux-core/Models.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ using namespace std;

namespace iptux {

// MARK: PalInfo

PalInfo::PalInfo(in_addr ipv4, uint16_t port)
: segdes(NULL), photo(NULL), sign(NULL), packetn(0), rpacketn(0) {
this->ipv4_ = ipv4;
Expand Down Expand Up @@ -120,6 +122,8 @@ string PalInfo::toString() const {
int(rpacketn), compatible, online, changed, in_blacklist);
}

// MARK: FileInfo

FileInfo::FileInfo()
: fileid(0),
packetn(0),
Expand Down Expand Up @@ -160,6 +164,17 @@ void FileInfo::ensureFilesizeFilled() {
filesize = afs.ftwsize(filepath);
}

bool FileInfo::operator==(const FileInfo& rhs) const {
const FileInfo& lhs = *this;
return lhs.fileid == rhs.fileid && lhs.packetn == rhs.packetn &&
lhs.fileattr == rhs.fileattr && lhs.filesize == rhs.filesize &&
lhs.finishedsize == rhs.finishedsize &&
lhs.filectime == rhs.filectime && lhs.filemtime == rhs.filemtime &&
lhs.filenum == rhs.filenum;
}

// MARK: MsgPara

MsgPara::MsgPara(CPPalInfo pal)
: stype(MessageSourceType::PAL),
btype(GROUP_BELONG_TYPE_REGULAR),
Expand All @@ -171,14 +186,65 @@ string MsgPara::getSummary() const {
if (this->dtlist.empty()) {
return _("Empty Message");
}
return this->dtlist[0].getSummary();
return this->dtlist[0]->getSummary();
}

// MARK: ChipData

shared_ptr<ChipData> ChipData::newTxtMsg(const std::string& text) {
return shared_ptr<ChipData>(new ChipData(text));
}

shared_ptr<ChipData> ChipData::newImgMsg(const std::string& text,
bool deleteFileAfterSent) {
auto res =
shared_ptr<ChipData>(new ChipData(MessageContentType::PICTURE, text));
res->deleteFileAfterSent = deleteFileAfterSent;
Comment on lines +200 to +202
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Consider using make_shared for shared_ptr creation

Using make_shared is generally more efficient and safer than using new with shared_ptr. It performs a single memory allocation for both the control block and the object.

Suggested change
auto res =
shared_ptr<ChipData>(new ChipData(MessageContentType::PICTURE, text));
res->deleteFileAfterSent = deleteFileAfterSent;
auto res = make_shared<ChipData>(MessageContentType::PICTURE, text);
res->deleteFileAfterSent = deleteFileAfterSent;

return res;
}

ChipData::ChipData(const string& data)
: type(MessageContentType::STRING), data(data) {}
ChipData::ChipData(MessageContentType type, const string& data)
: type(type), data(data) {}
ChipData::~ChipData() {}
ChipData::~ChipData() {
if (type == MessageContentType::PICTURE && deleteFileAfterSent) {
g_unlink(data.c_str());
}
}

string ChipData::ToString() const {
ostringstream oss;
oss << "ChipData(";
switch (type) {
case MessageContentType::STRING:
oss << "MessageContentType::STRING";
break;
case MessageContentType::PICTURE:
oss << "MessageContentType::PICTURE";
break;
default:
g_assert_not_reached();
}
oss << ", ";
oss << data;
oss << ")";
return oss.str();
}

string ChipData::getSummary() const {
switch (type) {
case MessageContentType::STRING:
return data;
case MessageContentType::PICTURE:
return _("Received an image");
default:
g_assert_not_reached();
}
return "";
}

// MARK: NetSegment

NetSegment::NetSegment() {}

Expand Down Expand Up @@ -223,36 +289,7 @@ NetSegment NetSegment::fromJsonValue(Json::Value& value) {
return res;
}

string ChipData::ToString() const {
ostringstream oss;
oss << "ChipData(";
switch (type) {
case MessageContentType::STRING:
oss << "MessageContentType::STRING";
break;
case MessageContentType::PICTURE:
oss << "MessageContentType::PICTURE";
break;
default:
g_assert_not_reached();
}
oss << ", ";
oss << data;
oss << ")";
return oss.str();
}

string ChipData::getSummary() const {
switch (type) {
case MessageContentType::STRING:
return data;
case MessageContentType::PICTURE:
return _("Received an image");
default:
g_assert_not_reached();
}
return "";
}
// MARK: PalKey

PalKey::PalKey(in_addr ipv4, int port) : ipv4(ipv4), port(port) {}

Expand All @@ -268,13 +305,4 @@ string PalKey::ToString() const {
return stringFormat("%s:%d", inAddrToString(ipv4).c_str(), port);
}

bool FileInfo::operator==(const FileInfo& rhs) const {
const FileInfo& lhs = *this;
return lhs.fileid == rhs.fileid && lhs.packetn == rhs.packetn &&
lhs.fileattr == rhs.fileattr && lhs.filesize == rhs.filesize &&
lhs.finishedsize == rhs.finishedsize &&
lhs.filectime == rhs.filectime && lhs.filemtime == rhs.filemtime &&
lhs.filenum == rhs.filenum;
}

} // namespace iptux
15 changes: 8 additions & 7 deletions src/iptux-core/ModelsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,14 @@ TEST(NetSegment, ContainIP) {
}

TEST(ChipData, ToString) {
EXPECT_EQ(ChipData("").ToString(), "ChipData(MessageContentType::STRING, )");
EXPECT_EQ(ChipData::newTxtMsg("")->ToString(),
"ChipData(MessageContentType::STRING, )");
}

TEST(ChipData, getSummary) {
EXPECT_EQ(ChipData("").getSummary(), "");
EXPECT_EQ(ChipData("foobar").getSummary(), "foobar");
EXPECT_EQ(ChipData(MessageContentType::PICTURE, "foobar").getSummary(),
EXPECT_EQ(ChipData::newTxtMsg("")->getSummary(), "");
EXPECT_EQ(ChipData::newTxtMsg("foobar")->getSummary(), "foobar");
EXPECT_EQ(ChipData::newImgMsg("foobar", false)->getSummary(),
"Received an image");
}

Expand All @@ -72,12 +73,12 @@ TEST(MsgPara, getSummary) {
auto pal = make_shared<PalInfo>("127.0.0.1", 2425);
MsgPara msg(pal);
EXPECT_EQ(msg.getSummary(), "Empty Message");
msg.dtlist.push_back(ChipData("foobar"));
msg.dtlist.push_back(ChipData::newTxtMsg("foobar"));
EXPECT_EQ(msg.getSummary(), "foobar");
msg.dtlist.push_back(ChipData("foobar2"));
msg.dtlist.push_back(ChipData::newTxtMsg("foobar2"));
EXPECT_EQ(msg.getSummary(), "foobar");
msg.dtlist.clear();
msg.dtlist.push_back(ChipData(MessageContentType::PICTURE, "foobar"));
msg.dtlist.push_back(ChipData::newImgMsg("foobar", false));
EXPECT_EQ(msg.getSummary(), "Received an image");
}

Expand Down
2 changes: 1 addition & 1 deletion src/iptux-core/internal/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ void Command::FeedbackError(CPPalInfo pal,
para.stype = MessageSourceType::ERROR;
para.btype = btype;

ChipData chip(MESSAGE_CONTENT_TYPE_STRING, error);
auto chip = ChipData::newTxtMsg(error);
para.dtlist.push_back(std::move(chip));
/* 交给某人处理吧 */
coreThread.InsertMessage(std::move(para));
Expand Down
3 changes: 1 addition & 2 deletions src/iptux-core/internal/TcpData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,7 @@ void TcpData::RecvMsgPic(PalInfo* pal, const char* path) {
/* 构建消息封装包 */
para.stype = MessageSourceType::PAL;
para.btype = GROUP_BELONG_TYPE_REGULAR;
ChipData chip(MESSAGE_CONTENT_TYPE_PICTURE, path);
para.dtlist.push_back(chip);
para.dtlist.push_back(ChipData::newImgMsg(path, false));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (performance): Consider using emplace_back

Consider using emplace_back instead of push_back to construct the object in place and avoid an unnecessary copy.

Suggested change
para.dtlist.push_back(ChipData::newImgMsg(path, false));
para.dtlist.emplace_back(ChipData::newImgMsg(path, false));


/* 交给某人处理吧 */
coreThread->InsertMessage(std::move(para));
Expand Down
3 changes: 2 additions & 1 deletion src/iptux-core/internal/UdpData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <glib/gi18n.h>

#include "iptux-core/CoreThread.h"
#include "iptux-core/Models.h"
#include "iptux-core/internal/Command.h"
#include "iptux-core/internal/CommandMode.h"
#include "iptux-core/internal/RecvFile.h"
Expand Down Expand Up @@ -515,7 +516,7 @@ void UdpData::InsertMessage(PPalInfo pal,
/* 构建消息封装包 */
para.stype = MessageSourceType::PAL;
para.btype = btype;
ChipData chip(MESSAGE_CONTENT_TYPE_STRING, msg);
auto chip = ChipData::newTxtMsg(msg);
para.dtlist.push_back(std::move(chip));

/* 交给某人处理吧 */
Expand Down
12 changes: 4 additions & 8 deletions src/iptux/DialogBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -391,17 +391,13 @@ bool DialogBase::SendEnclosureMsg() {
return true;
}

/**
* 回馈消息.
* @param msg 消息
*/
void DialogBase::FeedbackMsg(const gchar* msg) {
MsgPara para(this->app->getMe());
void DialogBase::FeedbackMsg(shared_ptr<MsgPara> msgPara) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Inconsistent naming convention

The method name FeedbackMsg does not follow the camelCase convention used elsewhere in the codebase. Consider renaming it to feedbackMsg for consistency.

MsgPara para(grpinf->getMembers()[0]);

para.stype = MessageSourceType::SELF;
para.btype = grpinf->getType();
para.dtlist = msgPara->dtlist;

ChipData chip(msg);
para.dtlist.push_back(std::move(chip));
grpinf->addMsgPara(para);
}

Expand Down
Loading
Loading