Skip to content

Conversation

@vranki
Copy link

@vranki vranki commented Jun 17, 2020

This seems to fix issues with CTCP actions containing ASCII control codes.

On my test setup there is no more false re-encoding for utf-8 actions like with is-utf8 library.
I suggest a new release to npm after this to get the fix usable in Matrix IRC bridge. At least
IRCnet bridge suffers from the encoding bugs.

lib/irc.js Outdated
if (!isUtf8(str)) {
const converter = new Iconv(self.opt.encodingFallback, "UTF-8")
if (!isValidUTF8(str)) {
const converter = new Iconv(self.opt.encodingFallback[0], "UTF-8")
Copy link

Choose a reason for hiding this comment

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

Did you mean to change this to treating encodingFallback as an array? It's defined as a string in matrix-appservice-irc.

Copy link
Author

Choose a reason for hiding this comment

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

Hm, I suppose not. Need to check that again.

@ilmari
Copy link

ilmari commented Jun 22, 2020

@vranki ping? Also, the CI build is failing:

$ npm ci
npm ERR! cipm can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with npm install before continuing.
npm ERR!
npm ERR!
npm ERR! Missing: utf-8-validate@^5.0.2
npm ERR!

@vranki
Copy link
Author

vranki commented Jun 22, 2020

I'm on a business trip so can't do much right now. Anyone is free to make the required changes based on this pr.

Copy link

@ilmari ilmari left a comment

Choose a reason for hiding this comment

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

LGTM

@Half-Shot Half-Shot merged commit 9028c21 into matrix-org:master Jul 9, 2020
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.

3 participants