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

fix(wp): handling of CR characters #2189

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

fix(wp): handling of CR characters #2189

wants to merge 1 commit into from

Conversation

sjinks
Copy link
Member

@sjinks sjinks commented Jan 11, 2025

Description

This PR strips CR characters from the input stream, making --prompt option work.

Ref: pbq1wL-1LP#comment-1898

Pull request checklist

New release checklist

Steps to Test

Try vip @env -y -- wp option update --prompt command to ensure they work.

@sjinks sjinks self-assigned this Jan 11, 2025
Copy link
Contributor

github-actions bot commented Jan 11, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Files

@sjinks
Copy link
Member Author

sjinks commented Jan 11, 2025

The alternative is to ignore CR characters in cron-control-runner's TTY:

diff --git a/remote/remote.go b/remote/remote.go
index 4e81932..7fdabb7 100644
--- a/remote/remote.go
+++ b/remote/remote.go
@@ -32,6 +32,7 @@ import (
        "github.com/hashicorp/go-retryablehttp"
        "github.com/howeyc/fsnotify"
        "golang.org/x/net/websocket"
+       "golang.org/x/sys/unix"
        "golang.org/x/term"
 )
 
@@ -440,6 +441,16 @@ func processShutdown(conn net.Conn, wpcli *wpCLIProcess) {
        wpcli.padlock.Unlock()
 }
 
+func setPtyToIgnoreCR(fd int) error {
+       termios, err := unix.IoctlGetTermios(fd, unix.TCGETS)
+       if err == nil {
+               termios.Iflag |= unix.IGNCR
+               return unix.IoctlSetTermios(fd, unix.TCSETS, termios)
+       }
+
+       return err
+}
+
 func processTCPConnectionData(conn net.Conn, wpcli *wpCLIProcess) {
        data := make([]byte, 8192)
        var size, written int
@@ -775,6 +786,10 @@ func runWpCliCmdRemote(conn net.Conn, GUID string, rows uint16, cols uint16, wpC
        }
        defer func() { _ = term.Restore(int(tty.Fd()), prevState) }()
 
+       if e := setPtyToIgnoreCR(int(tty.Fd())); e != nil {
+               return fmt.Errorf("runWpCliCmdRemote: error setting the WP CLI tty to send LF: %s", e.Error())
+       }
+
        readFile, err := os.OpenFile(logFileName, os.O_RDONLY, os.ModeCharDevice)
        if nil != err {
                conn.Close()

@sjinks sjinks changed the title fix(wp): handling of CR caharacters fix(wp): handling of CR characters Jan 11, 2025
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.

1 participant