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

fix weird n-fold unescaping in expand_escapes #723

Merged
merged 2 commits into from Jun 23, 2017

Conversation

Projects
None yet
3 participants
@ailin-nemui
Contributor

ailin-nemui commented Jun 19, 2017

noticed by @osm in #709

@osm

This comment has been minimized.

Contributor

osm commented Jun 21, 2017

I tested this together with the PR in #709 and it works correctly.

@dequis

This looked like some sort of regression with #520 / #669 but that just makes it more visible, and I can reproduce this with 0.8.15:

foo \x5cx5c \0 -> foo \x5c
foo \x5cx5c \n -> foo \

Took me a bit to get the issue. Explaining, for the record: expand_escapes goes over every escape of the string, writes the result to a temp string, and when it finds a newline it calls event_text/the send text signal. The signal handler (in the same file) calls expand_escapes again. The old code used the temp string up to the point it escaped, the new code copies the relevant section from the original string so that it does the escaping a second time but over a string that is clean and ends right before the newline.

@@ -1036,9 +1040,11 @@ static char *expand_escapes(const char *line, SERVER_REC *server,
/* newline .. we need to send another "send text"
event to handle it (or actually the text before
the newline..) */
if (ret != ptr) {
*ptr = '\0';
signal_emit("send text", 3, ret, server, item);

This comment has been minimized.

@dequis

dequis Jun 21, 2017

Member

What things are not being called when removing this? Anything in particular you're avoiding?

It looks like fe-exec has a send text handler for writing into exec window items

This comment has been minimized.

@ailin-nemui

ailin-nemui Jun 22, 2017

Contributor

in any case calling "send text" again to implement this "hack" would mean
"send text" "abc\ndef"
"send text" "abc"
being emitted, which seems wrong. the fe-text handler on the other hand catches the send text first and thus expand_escapes doesn't work at all (also sad ;-(

This comment has been minimized.

@dequis

dequis Jun 23, 2017

Member

Hmm. It's not affecting escapes at all in fe-exec either before or after this change. I guess this is fine.

*ptr = '\0';
signal_emit("send text", 3, ret, server, item);
if (prev != line) {
const char *prev_line = g_strndup(prev, (line - prev) - 1);

This comment has been minimized.

@dequis

dequis Jun 21, 2017

Member

Remove const? g_strndup returns char *, it's being casted back to char below, and it's ok to promote non-const to const.

@ailin-nemui ailin-nemui removed the auto-merge label Jun 22, 2017

@dequis

dequis approved these changes Jun 23, 2017

@ailin-nemui ailin-nemui merged commit 1ff2f61 into irssi:master Jun 23, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ailin-nemui ailin-nemui deleted the ailin-nemui:odd_expand_escapes branch Jun 25, 2017

ailin-nemui added a commit to ailin-nemui/irssi that referenced this pull request Dec 7, 2017

Merge pull request irssi#723 from ailin-nemui/odd_expand_escapes
fix weird n-fold unescaping in expand_escapes

(cherry picked from commit 1ff2f61)

@ailin-nemui ailin-nemui added this to the 1.0.4 milestone Jan 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment