Skip to content

feat: enable libcurl for znc-push#55

Closed
mbologna wants to merge 3 commits intolinuxserver:masterfrom
mbologna:feat/enable_curl_for_znc-push
Closed

feat: enable libcurl for znc-push#55
mbologna wants to merge 3 commits intolinuxserver:masterfrom
mbologna:feat/enable_curl_for_znc-push

Conversation

@mbologna
Copy link
Copy Markdown

@mbologna mbologna commented Feb 25, 2025

linuxserver.io


  • I have read the contributing guideline and understand that I have made the correct modifications

Description:

Before enabling libcurl in znc-push, some notifications were not delivered, especially messages that were sent in burst.
Working example:

<*push> Building notification to
api.telegram.org/botXXX/sendMessage...
<*push> Request sending
<*push> Status: 200
<*push> Message: OK
...

Broken example:

<*push> Building notification to
api.telegram.org/botXXX/sendMessage...
<*push> Request sending
...
<Notification not delivered>

Ref: https://github.com/amyreese/znc-push/blob/master/README.md#advanced

Note: You are strongly encouraged to use libcurl transport. The reason for that is, that the default CSocket transport doesn't verify server's SSL certificate which leaves you vulnerable to MITM attacks. However, use of libcurl will block the main ZNC thread at every push notification; for installations with many users, libcurl is not yet ideal, even with the above security concerns in mind.

I subsequently enabled libcurl in push.cpp compilation.
I could not modify the Makefile because it is overwritten for every module downloaded, but I could override the cpp file before building the module.
After using libcurl, I could receive all the notifications sent by znc-push.

Benefits of this PR and context:

Make znc-push use libcurl for reliable notifications

How Has This Been Tested?

Rebuilt the docker-znc image and tested it is working: znc-push is using libcurl (/msg *push set debug on)

Source / References:

https://github.com/amyreese/znc-push/blob/master/README.md#advanced

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Thanks for opening this pull request! Be sure to follow the pull request template!

@mbologna mbologna force-pushed the feat/enable_curl_for_znc-push branch from 8080b98 to ece4d55 Compare February 25, 2025 10:38
@LinuxServer-CI
Copy link
Copy Markdown
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-60b10c21-dev-ece4d5563fdfef1414b4e453f6232c31908b0e19-pr-55/index.html
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-60b10c21-dev-ece4d5563fdfef1414b4e453f6232c31908b0e19-pr-55/shellcheck-result.xml

Tag Passed
amd64-znc-1.9.1-pkg-60b10c21-dev-ece4d5563fdfef1414b4e453f6232c31908b0e19-pr-55
arm64v8-znc-1.9.1-pkg-60b10c21-dev-ece4d5563fdfef1414b4e453f6232c31908b0e19-pr-55

@mbologna mbologna force-pushed the feat/enable_curl_for_znc-push branch from ece4d55 to da675bf Compare February 25, 2025 12:38
Before enabling libcurl in znc-push, some notifications were not
delivered, especially messages that were sent in burst.
Working example:

```
<*push> Building notification to
api.telegram.org/botXXX/sendMessage...
<*push> Request sending
<*push> Status: 200
<*push> Message: OK
...
```

Broken example:

```
<*push> Building notification to
api.telegram.org/botXXX/sendMessage...
<*push> Request sending
...
<Notification not delivered>
```

Ref: https://github.com/amyreese/znc-push/blob/master/README.md#advanced
```
Note: You are strongly encouraged to use libcurl transport. The reason
for that is, that the default CSocket transport doesn't verify server's
SSL certificate which leaves you vulnerable to MITM attacks. However,
use of libcurl will block the main ZNC thread at every push
notification; for installations with many users, libcurl is not yet
ideal, even with the above security concerns in mind.
```

I subsequently enabled libcurl in push.cpp compilation.
I could not modify the Makefile because it is overwritten for every
module downloaded, but I could override the cpp file before building the
module.
After using libcurl, I could receive all the notifications sent by
znc-push.
@mbologna mbologna force-pushed the feat/enable_curl_for_znc-push branch from da675bf to 6fc8a03 Compare February 25, 2025 12:40
@LinuxServer-CI
Copy link
Copy Markdown
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-60b10c21-dev-da675bfd6e1629f1c51b571218beb40e26b7bb6b-pr-55/index.html
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-60b10c21-dev-da675bfd6e1629f1c51b571218beb40e26b7bb6b-pr-55/shellcheck-result.xml

Tag Passed
amd64-znc-1.9.1-pkg-60b10c21-dev-da675bfd6e1629f1c51b571218beb40e26b7bb6b-pr-55
arm64v8-znc-1.9.1-pkg-60b10c21-dev-da675bfd6e1629f1c51b571218beb40e26b7bb6b-pr-55

@LinuxServer-CI
Copy link
Copy Markdown
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-60b10c21-dev-6fc8a03907c2354ea96618a17d1bb673c3cf68af-pr-55/index.html
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-60b10c21-dev-6fc8a03907c2354ea96618a17d1bb673c3cf68af-pr-55/shellcheck-result.xml

Tag Passed
amd64-znc-1.9.1-pkg-60b10c21-dev-6fc8a03907c2354ea96618a17d1bb673c3cf68af-pr-55
arm64v8-znc-1.9.1-pkg-60b10c21-dev-6fc8a03907c2354ea96618a17d1bb673c3cf68af-pr-55

@LinuxServer-CI
Copy link
Copy Markdown
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-a136393e-dev-fe69e7a1c2e112c00391c91bf2dde04a34085779-pr-55/index.html
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-a136393e-dev-fe69e7a1c2e112c00391c91bf2dde04a34085779-pr-55/shellcheck-result.xml

Tag Passed
amd64-znc-1.9.1-pkg-a136393e-dev-fe69e7a1c2e112c00391c91bf2dde04a34085779-pr-55
arm64v8-znc-1.9.1-pkg-a136393e-dev-fe69e7a1c2e112c00391c91bf2dde04a34085779-pr-55

@LinuxServer-CI
Copy link
Copy Markdown
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-a136393e-dev-42b066dff99a7158a2b5747ddc77b51109a80c31-pr-55/index.html
https://ci-tests.linuxserver.io/lspipepr/znc/znc-1.9.1-pkg-a136393e-dev-42b066dff99a7158a2b5747ddc77b51109a80c31-pr-55/shellcheck-result.xml

Tag Passed
amd64-znc-1.9.1-pkg-a136393e-dev-42b066dff99a7158a2b5747ddc77b51109a80c31-pr-55
arm64v8-znc-1.9.1-pkg-a136393e-dev-42b066dff99a7158a2b5747ddc77b51109a80c31-pr-55

@Roxedus
Copy link
Copy Markdown
Member

Roxedus commented Mar 10, 2025

We don't know how many users our image consumers has, and the disclaimer for libcurl locking ZNC is not really worth lessening the experience for others.

use of libcurl will block the main ZNC thread at every push notification

@Roxedus Roxedus closed this Mar 10, 2025
@LinuxServer-CI LinuxServer-CI moved this from PRs to Done in Issue & PR Tracker Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants