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

[Minor] rspamc: fix crash on non-string element in messages #4214

Merged
merged 1 commit into from
Jul 15, 2022

Conversation

fatalbanana
Copy link
Member

Lists and dictionaries can be added to the protocol response inside messages, e.g.

          task:append_message({foo = 'bar'}, 'yolo')
          task:append_message({'bar','foo'}, 'swag')

Currently this is causing a crash in rspamc.

(gdb) p ucl_object_tostring(cmesg)
$3 = 0x0

This doesn't look quite as I might have imagined it but it's better than crashing:

Message - yolo: object
Message - swag: array

@fatalbanana fatalbanana changed the title [Minor] rspamc: fix crash on non-string element in messages WIP:[Minor] rspamc: fix crash on non-string element in messages Jul 13, 2022
@fatalbanana
Copy link
Member Author

the patch as currently proposed mishandles strings. :\ I'll revisit.

@fatalbanana
Copy link
Member Author

Grr, I was using a wrong API in testing (set_message where I meant append_message). :/ This is OK ...

@fatalbanana fatalbanana changed the title WIP:[Minor] rspamc: fix crash on non-string element in messages [Minor] rspamc: fix crash on non-string element in messages Jul 14, 2022
@vstakhov
Copy link
Member

I don't like the usage of the ucl_object_tostring_forced here, as it is way too bogus. For a human readable implementation I would suggest just to emit ucl object into, well, ucl. ucl_object_tostring_forced was designed to do something similar but in a more intrusive and state modifying way: deconst->trash_stack[UCL_TRASH_VALUE] = ucl_object_emit_single_json (obj);. Please note that it also emits json instead of a more logical choice of the UCL_EMIT_CONFIG (at least for the human readable output).

@fatalbanana
Copy link
Member Author

Message - yolo: {"foo":"bar"}
Message - swag: ["bar","foo"]
Message - barf: lol

looks nicer

src/client/rspamc.cxx Outdated Show resolved Hide resolved
src/client/rspamc.cxx Outdated Show resolved Hide resolved
src/client/rspamc.cxx Outdated Show resolved Hide resolved
@fatalbanana
Copy link
Member Author

Using UCL_EMIT_CONFIG as suggested:

Message - yolo: foo = "bar";

Message - swag: [
    "bar",
    "foo",
]

looks worse to me.

@vstakhov
Copy link
Member

Ok, let's go with the JSON approach, but please read my other comments.

@fatalbanana
Copy link
Member Author

Message - yolo: {"foo":"bar"}
Message - swag: ["bar","foo"]
Message - long: ["long","long","long","long","long","long","long","long","lo
Message - barf: lol

@vstakhov vstakhov merged commit 1aef384 into rspamd:master Jul 15, 2022
@fatalbanana fatalbanana deleted the rspamc_messages branch July 15, 2022 13:41
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.

None yet

2 participants