Skip to content

fix(process): improve stdout encoding handling in read_line functions #1471 - #3148

Closed
Tunglies wants to merge 5 commits into
tauri-apps:v2from
Tunglies:fix-1471-stdout-encoding
Closed

Tunglies wants to merge 5 commits into
tauri-apps:v2from
Tunglies:fix-1471-stdout-encoding

Conversation

@Tunglies

@Tunglies Tunglies commented Dec 5, 2025

Copy link
Copy Markdown
Contributor

I developed on macOS, need to make sure encoding behavior same as Windows.
Sidecar:

use encoding_rs;
use std::io::{self, Write};
use std::thread;
use std::time::Duration;

fn main() {
    let stdout = io::stdout();
    let mut handle = stdout.lock();
    let utf8_output = "[download] Destination: PEDRO - Jaxomy, Agatino Romero, Raffaella Carrà (TikTok Song) [XzilCu9PcZk].f616.mp4\n";
    const TARGET_ENCODING: &'static encoding_rs::Encoding = encoding_rs::WINDOWS_1252;
    let (encoded_bytes, _, malformed) = TARGET_ENCODING.encode(utf8_output);
    if malformed {
        eprintln!("[SIM] Warning: some characters cannot be encoded in Windows-1252!");
    }
    loop {
        match handle.write_all(encoded_bytes.as_ref()) {
            Ok(_) => {
                eprintln!(
                    "[SIM] Simulated output: successfully wrote {} bytes.",
                    encoded_bytes.len()
                );
            }
            Err(e) => {
                eprintln!("[SIM] Error writing to stdout: {}", e);
                break;
            }
        }
        thread::sleep(Duration::from_millis(1000));
    }
    let check_a_grave = TARGET_ENCODING.encode("à").0;
    eprintln!(
        "[SIM] Check: 'à' is encoded in Windows-1252 as bytes {:?}",
        check_a_grave
    );
}

User side, code snippet

let command = app_handle
            .clone()
            .shell()
            .sidecar(EXTERNAL_BIN)
            .expect("bin");
        let (mut rx, _) = command.spawn().expect("spawn");

        tauri::async_runtime::spawn(async move {
            while let Some(event) = rx.recv().await {
                if let CommandEvent::Stdout(line) = event {
                    let output = match String::from_utf8(line) {
                        Ok(s) => s,
                        Err(e) => {
                            println!("Invalid UTF-8 sequence: {}", e);
                            String::from_utf8_lossy(&e.into_bytes()).into_owned()
                        },
                    };
                    println!("{}", output);
                }
            }
        });

Output with mainline

[download] Destination: PEDRO - Jaxomy, Agatino Romero, Raffaella Carr� (TikTok Song) [XzilCu9PcZk].f616.mp4

Output with PR

[download] Destination: PEDRO - Jaxomy, Agatino Romero, Raffaella Carrà (TikTok Song) [XzilCu9PcZk].f616.mp4

@Tunglies
Tunglies requested a review from a team as a code owner December 5, 2025 10:10
@Tunglies

Tunglies commented Dec 5, 2025

Copy link
Copy Markdown
Contributor Author

CC @FabianLars, this is initial start. Potential to continue improve with your advice.

there's something for us to fix #1471 (comment)

If auto encoding is required, current hardcoded for encoding need change to system encoding page detect, and brings more cross-platform support.

@github-actions

github-actions Bot commented Dec 5, 2025 •

Copy link
Copy Markdown
Contributor

Package Changes Through f5858bf

There are 21 changes which include dialog-js with minor, dialog with minor, log with minor, log-js with minor, localhost with patch, barcode-scanner with patch, barcode-scanner-js with patch, deep-link with patch, deep-link-js with patch, shell with patch, shell-js with patch, http with patch, http-js with patch, nfc with patch, nfc-js with patch, updater with minor, updater-js with minor, upload with minor, upload-js with minor, websocket with patch, websocket-js with patch

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
api-example 2.0.38 2.0.39
api-example-js 2.0.34 2.0.35
deep-link-example-js 2.2.8 2.2.9
barcode-scanner 2.4.2 2.4.3
barcode-scanner-js 2.4.2 2.4.3
deep-link 2.4.5 2.4.6
deep-link-js 2.4.5 2.4.6
dialog 2.4.2 2.5.0
dialog-js 2.4.2 2.5.0
http 2.5.4 2.5.5
http-js 2.5.4 2.5.5
localhost 2.3.1 2.3.2
log 2.7.1 2.8.0
log-js 2.7.1 2.8.0
nfc 2.3.3 2.3.4
nfc-js 2.3.3 2.3.4
shell 2.3.3 2.3.4
shell-js 2.3.3 2.3.4
single-instance 2.3.6 2.3.7
updater 2.9.0 2.10.0
updater-js 2.9.0 2.10.0
upload 2.3.2 2.4.0
upload-js 2.3.2 2.4.0
websocket 2.4.1 2.4.2
websocket-js 2.4.1 2.4.2

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@Tunglies
Tunglies marked this pull request as draft December 5, 2025 10:40
@Tunglies
Tunglies force-pushed the fix-1471-stdout-encoding branch from f31eb16 to f5858bf Compare December 5, 2025 10:54
@Tunglies
Tunglies marked this pull request as ready for review December 5, 2025 10:54
@FabianLars

Copy link
Copy Markdown
Member

@Legend-Master Do you know if this could also cause issues? This feels like it depends on the spawned process eg iirc node also uses utf8 on all platforms by default 🤔

@Legend-Master

Copy link
Copy Markdown
Contributor

Since this is the raw output of the command in Vec<u8>, I don't think we should alter/preprocess it here to be honest

And for the encoding, I think it really depends on the command/program you spawned, it's not something we could assume (by the way, windows-1252 code page / encoding isn't the one used in all applications, it's only for apps using ANSI with ANSI Latin 1; Western European (Windows))

https://learn.microsoft.com/en-us/windows/win32/intl/code-page-identifiers

@Tunglies

Tunglies commented Dec 7, 2025

Copy link
Copy Markdown
Contributor Author

I found re-export usage use tauri_plugin_shell::process::Encoding;, use that we can do

let decoder = encoding_rs::WINDOWS_1252;
let (cow, _, had_errors) = recoder.decode(&line);
let output: String = cow.into_owned();

it's not something we could assume

Shall we leave this and documents on the usage of re-export Encoding? #1471 isn't a bug, user should consider encoding when use.

@Legend-Master

Copy link
Copy Markdown
Contributor

Our JavaScript API defaulting to assume UTF-8 which I think is more or less in line with the Rust behavior (since JS one defaults to string while the Rust one is Vec<u8>), I think maybe we mirror the SpawnOptions.encoding option from JS? (and of course documentations)

#1471 isn't a bug, user should consider encoding when use.

Yeah, the program can output whatever they want technically, we can only provide sensible defaults that works for most

@Tunglies

Tunglies commented Dec 7, 2025

Copy link
Copy Markdown
Contributor Author
sturcture Command {
 encoding: Encoding
 ...
}
app_handle.sidecar(BIN).output_line_encoding(Encoding::Type)

Some thing like this? The .spawn only returns rx and child. Additional field of Command structure needed. And this should only takes effect on not-raw output.

@Legend-Master

Copy link
Copy Markdown
Contributor

Ah, I see, let's just do the documentation work for now then

@Tunglies

Copy link
Copy Markdown
Contributor Author

The user should considering encoding on their own. Shold I close this PR or leave as reference?

@Legend-Master

Copy link
Copy Markdown
Contributor

We could close it or turn it into a PR that updates the documentation or if you want to open a new one that's fine too

@Legend-Master

Legend-Master commented Dec 29, 2025 •

Copy link
Copy Markdown
Contributor

Documentation added in #3183, thanks

@Tunglies
Tunglies deleted the fix-1471-stdout-encoding branch December 29, 2025 06:33
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