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

Add support for formatted test #122

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

EionRobb
Copy link
Collaborator

(Got a little annoyed at copy-pasting tags into messages and then getting stuck, but then took it a whole bunch further...)

This PR uses the markdown conversion code from purple-discord, tweaked for Slack's flavour of markdown, and uses it to convert to/from HTML (or specifically, Pidgin's HTML)

Would be good to get some tests/feedback on it :)

@ghost
Copy link

ghost commented Sep 30, 2020

Very much appreciated! However, I got a crash when I used this and tried to send a message. I used an @ reference and two strings in single backticks.

#0  0x00007ffff4e86f47 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff4e888b1 in __GI_abort () at abort.c:79
#2  0x00007ffff4ed1907 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7ffff4ffedfa "%s\n") at ../sysdeps/posix/libc_fatal.c:181
#3  0x00007ffff4ed897a in malloc_printerr (str=str@entry=0x7ffff5000e48 "malloc(): smallbin double linked list corrupted") at malloc.c:5350
#4  0x00007ffff4edcd54 in _int_malloc (av=av@entry=0x7ffff5233c40 <main_arena>, bytes=bytes@entry=512) at malloc.c:3648
#5  0x00007ffff4edf35d in __GI___libc_malloc (bytes=512) at malloc.c:3065
#6  0x00007ffff655f219 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#7  0x00007ffff65c3856 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#8  0x00007ffff65c4001 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#9  0x00007ffff65b9635 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#10 0x00007ffff65a2630 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#11 0x00007ffff65a270c in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#12 0x00007ffff65a31d8 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#13 0x00007ffff6546340 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#14 0x00007ffff65bfac0 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#15 0x00007ffff65903ba in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#16 0x00007ffff654e5b6 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#17 0x00007ffff6547b39 in  () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#18 0x00007ffff6540a55 in cairo_fill () at /usr/lib/x86_64-linux-gnu/libcairo.so.2
#19 0x00007fffe318e346 in  () at /usr/lib/x86_64-linux-gnu/gtk-2.0/2.10.0/engines/libpixmap.so
#20 0x00007fffe318f319 in  () at /usr/lib/x86_64-linux-gnu/gtk-2.0/2.10.0/engines/libpixmap.so
#21 0x00007fffe318d583 in  () at /usr/lib/x86_64-linux-gnu/gtk-2.0/2.10.0/engines/libpixmap.so
#22 0x00007fffe318dac2 in  () at /usr/lib/x86_64-linux-gnu/gtk-2.0/2.10.0/engines/libpixmap.so
#23 0x00007ffff6e67e18 in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#24 0x00007ffff6e4a38b in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#25 0x00007ffff5e6b021 in g_closure_invoke () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#26 0x00007ffff5e7dde8 in  () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#27 0x00007ffff5e860af in g_signal_emit_valist () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#28 0x00007ffff5e8712f in g_signal_emit () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#29 0x00007ffff6f602bc in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#30 0x00007ffff6dd090e in gtk_container_propagate_expose () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#31 0x00007ffff6d9bc85 in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#32 0x00007ffff6dcf38e in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#33 0x00007ffff6e4a38b in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#34 0x00007ffff5e6b021 in g_closure_invoke () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#35 0x00007ffff5e7dde8 in  () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#36 0x00007ffff5e860af in g_signal_emit_valist () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#37 0x00007ffff5e8712f in g_signal_emit () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#38 0x00007ffff6f602bc in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#39 0x00007ffff6dd090e in gtk_container_propagate_expose () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#40 0x00007ffff6dcf38e in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#41 0x00007ffff6e4a38b in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#42 0x00007ffff5e6b10d in g_closure_invoke () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#43 0x00007ffff5e7dde8 in  () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#44 0x00007ffff5e860af in g_signal_emit_valist () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#45 0x00007ffff5e8712f in g_signal_emit () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
---Type <return> to continue, or q <return> to quit---
#46 0x00007ffff6f602bc in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#47 0x00007ffff6e48c68 in gtk_main_do_event () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#48 0x00007ffff6aa4b9f in  () at /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
#49 0x00007ffff6aa1623 in  () at /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
#50 0x00007ffff6aa1fb0 in gdk_window_process_all_updates () at /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
#51 0x00007ffff6dcf061 in  () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#52 0x00007ffff6a80c1c in  () at /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
#53 0x00007ffff5b90285 in g_main_context_dispatch () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#54 0x00007ffff5b90650 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#55 0x00007ffff5b90962 in g_main_loop_run () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#56 0x00007ffff6e47a37 in gtk_main () at /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#57 0x0000000000499aa4 in main (argc=1, argv=0x7fffffffde58) at gtkmain.c:939

It looks like it's in a different section, so probably the memory error has already happened sometime earlier.

@EionRobb
Copy link
Collaborator Author

Yeah, since posting the PR, I've been seeing approx 5% of sent messages having some kind of memory error - either an outright crash, or the message body gets replaced with the channel ID - but when I step through with gdb everything looks ok (stupid heisenbugs!)

I don't have access to valgrind (since I'm on Windows) but do you think you could run Pidgin through valgrind @kacf to see if it reveals where the corruption is happening?

The other bug I'm seeing after this PR is that incoming smileys which have underscores in them, eg :slightly_smiling_face: are converted into italics incorrectly, eg :slightly<i>smiling</i>face: which is because the markdown parser follows discord's type of markdown for __ vs _ which Slack doesn't differentiate

@dylex
Copy link
Owner

dylex commented Oct 5, 2020

In a very cursory try I'm getting:

==196083== Invalid free() / delete / delete[] / realloc()
==196083==    at 0x4C2B06D: free (vg_replace_malloc.c:540)
==196083==    by 0x60B27ED: g_free (in /usr/lib64/libglib-2.0.so.0.5600.1)
==196083==    by 0xAD2309C: slack_html_to_message (slack-message.c:76)
==196083==    by 0xAD2734C: slack_send_im (slack-im.c:162)
==196083==    by 0x5754764: serv_send_im (server.c:145)
==196083==    by 0x572D766: common_send (conversation.c:182)
==196083==    by 0x41EA98: entry_key_pressed (gntconv.c:200)
==196083==    by 0x5A19A67: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.5600.1)
==196083==    by 0x5A2C3D8: ??? (in /usr/lib64/libgobject-2.0.so.0.5600.1)
==196083==    by 0x5A340D0: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.5600.1)
==196083==    by 0x5A343BE: g_signal_emit (in /usr/lib64/libgobject-2.0.so.0.5600.1)
==196083==    by 0x4E4B4F8: gnt_entry_key_pressed (gntentry.c:871)
==196083==  Address 0xf2f4890 is 0 bytes inside a block of size 64 free'd
==196083==    at 0x4C2B06D: free (vg_replace_malloc.c:540)
==196083==    by 0x60B27ED: g_free (in /usr/lib64/libglib-2.0.so.0.5600.1)
==196083==    by 0xAD2D961: markdown_helper_replace (markdown.c:112)
==196083==    by 0xAD2DE66: markdown_html_to_markdown (markdown.c:224)
==196083==    by 0xAD22F6C: slack_html_to_message (slack-message.c:19)
==196083==    by 0xAD2734C: slack_send_im (slack-im.c:162)
==196083==    by 0x5754764: serv_send_im (server.c:145)
==196083==    by 0x572D766: common_send (conversation.c:182)
==196083==    by 0x41EA98: entry_key_pressed (gntconv.c:200)
==196083==    by 0x5A19A67: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.5600.1)
==196083==    by 0x5A2C3D8: ??? (in /usr/lib64/libgobject-2.0.so.0.5600.1)
==196083==    by 0x5A340D0: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.5600.1)
==196083==  Block was alloc'd at
==196083==    at 0x4C29EBD: malloc (vg_replace_malloc.c:308)
==196083==    by 0x4C2C210: realloc (vg_replace_malloc.c:836)
==196083==    by 0x60B2785: g_realloc (in /usr/lib64/libglib-2.0.so.0.5600.1)
==196083==    by 0x60CE4A3: ??? (in /usr/lib64/libglib-2.0.so.0.5600.1)
==196083==    by 0x60CE4F1: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.5600.1)
==196083==    by 0xAD2E019: markdown_escape_md (markdown.c:247)
==196083==    by 0xAD22F5F: slack_html_to_message (slack-message.c:18)
==196083==    by 0xAD2734C: slack_send_im (slack-im.c:162)
==196083==    by 0x5754764: serv_send_im (server.c:145)
==196083==    by 0x572D766: common_send (conversation.c:182)
==196083==    by 0x41EA98: entry_key_pressed (gntconv.c:200)
==196083==    by 0x5A19A67: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.5600.1)

Which looks like escaped is getting freed twice. Probably slack_html_to_message line 76 can just be removed.

@EionRobb
Copy link
Collaborator Author

EionRobb commented Oct 5, 2020

Thanks for the help @dylex and @kacf :)

I should have paid more attention to the function that does in-place modification, so added some safety to it (should upstream for the discord prpl too)

@ghost
Copy link

ghost commented Oct 6, 2020

Great! I will retry!

(sorry, I was planning to do the Valgrind thing, but time flies and dylex beat me to it!)

`markdown_html_to_markdown()` returns a malloc'ed variable and frees
the input variable. Therefore it is `marked` we need to free, not
`escaped`.

Signed-off-by: Kristian Amlie <[email protected]>
@ghost
Copy link

ghost commented Oct 7, 2020

It's still not quite right. The markdown_html_to_markdown function has somewhat surprising behavior, check out EionRobb#1, this fixed the problem for me.

On a side note, I could not get code blocks to render any differently than normal text, but I can see from the log that the correct HTML is indeed there, so this may be a Pidgin issue, and not a Slack plugin issue.

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