Skip to content

GxsNetService: skip groups too large to fit in a packet - #392

Open
defnax wants to merge 3 commits into
RetroShare:masterfrom
defnax:fix-oversized-packets
Open

defnax wants to merge 3 commits into
RetroShare:masterfrom
defnax:fix-oversized-packets

Conversation

@defnax

@defnax defnax commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

RsNxsGrp items larger than the max packet size (262143 bytes) can never be serialised by pqistreamer. fragmentGrp() is not used and receivers do not defragment groups, so sending such a group only fails with "Serialised packet is too big", and the peer requests the same group again at every sync.

###################### RsRawItem #####################

(EE) p3FastService::sendItem(): OVERSIZED ITEM of 1198413 bytes.
     item class : 8RsNxsGrp
     packet id : 0x2021704
     service type : 0x217 (service, from packet id)
     sub type : 0x4
     destination : xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
(EE) Serialised packet is too big. Maximum allowed size is 262143. Serialised size is 1198413. Please tune your service to correctly split packets
(Mon Sep 21 21:59:25 2026 Z: pqistreamer, lvl: 0): pqistreamer::queue_outpqi() Null Pkt generated!
Caused By:
RsItem: RsRawItem ####################################
Size: 1198413

defnax and others added 2 commits September 21, 2026 22:28
RsNxsGrp items larger than the max packet size (262143 bytes) can never
be serialised by pqistreamer. fragmentGrp() is not used and receivers do
not defragment groups, so sending such a group only fails with
"Serialised packet is too big", and the peer requests the same group
again at every sync.
…e scope

Follow-up to the oversized-group skip: compare the exact serialised size
of the RsNxsGrp, as RsNxsSerialiser computes it, against a file-scope
MAX_ALLOWED_GXS_GROUP_SIZE placed next to MAX_ALLOWED_GXS_MESSAGE_SIZE.
Drop the NULL check and the manual delete: retrieveNxsGrps() erases the
ids it cannot find, and the temporary map deletes what is left in it.
@jolavillette

Copy link
Copy Markdown
Contributor

Went through it against master. The premise holds: NXS_FRAG is off, fragmentGrp() has no caller, and pqistreamer just drops anything above RsSerialiser::MAX_SERIAL_SIZE. So merging this makes sense, and the nItems change is actually the important part: the client only completes a GRPS transaction when it has received exactly nItems items (rsgxsnetservice.cc:2441), so today an oversized group also blocks every other group bundled in the same transaction for the 2000 s timeout, after which the whole transaction is wasted. With the PR the other groups get through.

Three small things, as a PR on your branch so you can just merge it if you agree: defnax#10.

  1. MAX_GRP_SEND_SIZE moved to file scope next to MAX_ALLOWED_GXS_MESSAGE_SIZE (line 309), which is the same kind of limit for messages, so both are found together.
  2. The size is the exact serialised size from RsNxsSerialiser::size() instead of grp.TlvSize() + meta.TlvSize().
  3. The delete / = NULL and the NULL check are not needed: retrieveNxsGrps() erases the ids it cannot find, and grps is a t_RsGxsGenericDataTemporaryMap whose destructor frees whatever is left in it, so a plain continue does the job.

On the cause: a group this big can only have been created locally (a receiving node closes the connection on a packet above 262143 bytes), and after this PR it is still advertised in the group list, so the peer will re-request it and you will get this new line once per minute per peer instead of the old dump. The check that removes the class of problem is at publication: publishGrps() only enforces GXS_MAX_ITEM_SIZE (1.5 MB), for groups and messages alike. I have that ready as a separate, complementary PR: a shared RS_GXS_MAX_GROUP_SIZE enforced in publishGrps() for creations and updates, plus the early size check with an explicit error that createBoardV2 already had, copied to createChannelV2. So creating such a group fails right away in whatever UI asked for it.

Speaking of which: 0x217 is channels, and the Qt GUI thumbnails the logo to 64×64, but the web UI sends the selected image file as is to createChannelV2 (channel_view.js:254), which had no size check. If that channel was created from the web UI, that is probably where the 1.2 MB comes from; the board file already has a resize helper for posts (preparePostImage) that could be reused for the channel and board image.

…-392

GxsNetService: measure the group with the serialiser, constant at file scope
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants