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

Foreground depends on a tty, results in no session password being logged #19984

Closed
Roxedus opened this issue Nov 21, 2023 · 8 comments · Fixed by #20018
Closed

Foreground depends on a tty, results in no session password being logged #19984

Roxedus opened this issue Nov 21, 2023 · 8 comments · Fixed by #20018
Labels
Milestone

Comments

@Roxedus
Copy link

Roxedus commented Nov 21, 2023

qBittorrent & operating system versions

qBittorrent: 4.6.1
Operating system: Ubuntu 20.04.5 LTS
Qt: 6.5.2-r0
libtorrent-rasterbar: info not present in SBOM

What is the problem?

This behavior has been present for a while, but has not caused any grief until 4.6.1 started logging the password in this area of the code.

The message that prints the temporary session password here does not run unless there is a tty present when it is running in the foreground, it is however shown when the application is told to shutdown (it presumably killed whatever was waiting/hanging on a tty) . The application does not show any other negative effects from running in the foreground without a tty (containers has done this for years).

Steps to reproduce

Run the official container without a tty and notice the lack of the A temporary password is provided for this session message.

docker run --rm --name=test -p 8080:8080 --tmpfs /config:exec ghcr.io/qbittorrent/docker-qbittorrent-nox:4.6.1-1

(ctrl + c starts the shutdown process, and you can now see the message getting logged)
then run the same image, but with a tty (-t) assigned.

docker run --rm -t --name=test -p 8080:8080 --tmpfs /config:exec ghcr.io/qbittorrent/docker-qbittorrent-nox:4.6.1-1

Additional context

No response

Log(s) & preferences file(s)

No response

@Roxedus Roxedus changed the title Foreground depends on a tty Foreground depends on a tty, results in no session password being logged Nov 21, 2023
@sledgehammer999
Copy link
Member

@glassez @Chocobo1

@sledgehammer999 sledgehammer999 added the WebUI WebUI-related issues/changes label Nov 21, 2023
@sledgehammer999 sledgehammer999 added this to the 4.6.2 milestone Nov 21, 2023
@Chocobo1
Copy link
Member

I'll take a look at it in the weekend.

@mariushosting
Copy link

Thank you @Roxedus !

@saltydk
Copy link

saltydk commented Nov 22, 2023

Turn off stdout buffering or insert a manual fflush on stdout in this section. Issue is likely that prints are no longer line buffered when there isn't an interactive device (tty).

On a related note this is the case for everything the WEB UI section prints, like this

@sledgehammer999
Copy link
Member

@Roxedus are you able to build/test yourself?
If so can you test the following patch?

diff --git a/src/app/application.cpp b/src/app/application.cpp
index e22f2f670..38df74aac 100644
--- a/src/app/application.cpp
+++ b/src/app/application.cpp
@@ -924,6 +924,7 @@ int Application::exec()
                     + tr("The WebUI administrator password was not set. A temporary password is provided for this session: %1").arg(tempPassword) + u'\n'
                     + tr("You should set your own password in program preferences.") + u'\n';
             printf("%s", qUtf8Printable(warning));
+            fflush(stdout); // Necessary for environments (e.g. docker) where stdout is buffered
         }
 #endif // DISABLE_GUI
 #endif // DISABLE_WEBUI

@Roxedus
Copy link
Author

Roxedus commented Nov 22, 2023

Yes, this patch works. (build)

@nephaste
Copy link

Same problem with static version : https://github.com/userdocs/qbittorrent-nox-static

Chocobo1 added a commit to Chocobo1/qBittorrent that referenced this issue Nov 25, 2023
The messages printed out via stdout is usually important and short so
there is no reason to buffer them.

Closes qbittorrent#19984.
@Chocobo1
Copy link
Member

I'll take a look at it in the weekend.

I have submitted a patch: #20018

Chocobo1 added a commit that referenced this issue Nov 26, 2023
The messages printed out via stdout is usually important and short so
there is no reason to buffer them.

Closes #19984.
PR #20018.
glassez pushed a commit to glassez/qBittorrent that referenced this issue Nov 26, 2023
The messages printed out via stdout is usually important and short so
there is no reason to buffer them.

Closes qbittorrent#19984.
PR qbittorrent#20018.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants