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.

Sign up for GitHub

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: actually give the larger incoming NGC custom packets via API to the client #2441

Closed
wants to merge 1 commit into from
Closed

fix: actually give the larger incoming NGC custom packets via API to the client #2441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1608abb52cd16775216d01d06569e6437d86ed11e9fc8dc6dc9c1c28668ccd8b /usr/local/bin/tox-bootstrapd
74c1b6175dc051e02c170ec8458c17fad3429534b2101c4ca7646ee645306cd9 /usr/local/bin/tox-bootstrapd
26 changes: 19 additions & 7 deletions toxcore/group_chats.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@ typedef enum Group_Sync_Flags {
GF_STATE = (1 << 2), // 4
} Group_Sync_Flags;

typedef enum Group_CustomPkts_Direction {
Copy link
Member

Choose a reason for hiding this comment

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

I dont think we need an whole enum for 2 states.

Copy link
Author

Choose a reason for hiding this comment

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

makes it more readable that way. with true/false you never know

Copy link
Member

Choose a reason for hiding this comment

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

still looks funky next to bool lossless, hmmm

Copy link
Member

Choose a reason for hiding this comment

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

+1 for enums, make the code much more readable.

Copy link
Member

Choose a reason for hiding this comment

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

Why Pkts and not just Packets?

GC_CUSTOMPKTS_SENDING = (1 << 0), // 1
Copy link
Member

Choose a reason for hiding this comment

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

Why are we using bitmasks? There will only ever be two possibilities and never at the same time. I don't mind an enum but the values shouldn't be bit set.

GC_CUSTOMPKTS_RECEIVING = (1 << 1), // 2
} Group_CustomPkts_Direction;

non_null() static bool self_gc_is_founder(const GC_Chat *chat);
non_null() static bool group_number_valid(const GC_Session *c, int group_number);
non_null() static int peer_update(const GC_Chat *chat, const GC_Peer *peer, uint32_t peer_number);
Expand Down Expand Up @@ -4906,11 +4911,17 @@ static int handle_gc_private_message(const GC_Session *c, const GC_Chat *chat, c
}

/** @brief Returns false if a custom packet is too large. */
static bool custom_gc_packet_length_is_valid(uint16_t length, bool lossless)
static bool custom_gc_packet_length_is_valid(uint16_t length, bool lossless, Group_CustomPkts_Direction pkt_direction)
Copy link
Member

Choose a reason for hiding this comment

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

This whole function should be rewritten with less nesting.

{
if (lossless) {
if (length > MAX_GC_CUSTOM_LOSSLESS_PACKET_SIZE) {
return false;
if (pkt_direction == GC_CUSTOMPKTS_SENDING) {
if (length > MAX_GC_CUSTOM_LOSSLESS_PACKET_SIZE) {
return false;
}
} else {
if (length > MAX_GC_PACKET_SIZE) {
return false;
}
}
} else {
if (length > MAX_GC_CUSTOM_LOSSY_PACKET_SIZE) {
Expand All @@ -4924,7 +4935,7 @@ static bool custom_gc_packet_length_is_valid(uint16_t length, bool lossless)
int gc_send_custom_private_packet(const GC_Chat *chat, bool lossless, uint32_t peer_id, const uint8_t *message,
uint16_t length)
{
if (!custom_gc_packet_length_is_valid(length, lossless)) {
if (!custom_gc_packet_length_is_valid(length, lossless, GC_CUSTOMPKTS_SENDING)) {
return -1;
}

Expand Down Expand Up @@ -4966,7 +4977,7 @@ non_null(1, 2, 3, 4) nullable(7)
static int handle_gc_custom_private_packet(const GC_Session *c, const GC_Chat *chat, const GC_Peer *peer,
const uint8_t *data, uint16_t length, bool lossless, void *userdata)
{
if (!custom_gc_packet_length_is_valid(length, lossless)) {
if (!custom_gc_packet_length_is_valid(length, lossless, GC_CUSTOMPKTS_RECEIVING)) {
return -1;
}

Expand All @@ -4987,7 +4998,7 @@ static int handle_gc_custom_private_packet(const GC_Session *c, const GC_Chat *c

int gc_send_custom_packet(const GC_Chat *chat, bool lossless, const uint8_t *data, uint16_t length)
{
if (!custom_gc_packet_length_is_valid(length, lossless)) {
if (!custom_gc_packet_length_is_valid(length, lossless, GC_CUSTOMPKTS_SENDING)) {
return -1;
}

Expand Down Expand Up @@ -5017,7 +5028,7 @@ non_null(1, 2, 3, 4) nullable(7)
static int handle_gc_custom_packet(const GC_Session *c, const GC_Chat *chat, const GC_Peer *peer, const uint8_t *data,
uint16_t length, bool lossless, void *userdata)
{
if (!custom_gc_packet_length_is_valid(length, lossless)) {
if (!custom_gc_packet_length_is_valid(length, lossless, GC_CUSTOMPKTS_RECEIVING)) {
return -1;
}

Expand Down Expand Up @@ -6057,6 +6068,7 @@ static bool handle_gc_lossless_packet(const GC_Session *c, GC_Chat *chat, const
uint8_t packet_type;
uint64_t message_id;


Copy link
Member

Choose a reason for hiding this comment

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

Remove double empty lines

const int len = group_packet_unwrap(chat->log, gconn, data, &message_id, &packet_type, packet, length);

if (len < 0) {
Expand Down
Loading