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

Limit number of read requests by other applications #22

Open
qjerome opened this issue Mar 4, 2022 · 8 comments
Open

Limit number of read requests by other applications #22

qjerome opened this issue Mar 4, 2022 · 8 comments
Labels
enhancement New feature or request platform-specific

Comments

@qjerome
Copy link

qjerome commented Mar 4, 2022

Hey @changkun,

Thank you for your great work.
I am wondering how hard it would be to implement the equivalent of the -l, -loops switch of xclip (X11 tool on Linux) in your package.
The goal would be to allow only a limited number of Read operations on the clipboard before the content gets flushed. I am currently relying on piping to xclip to do that but I think it is not very clean and I'd like to opt for a full C/Go approach.

Cheers,

@changkun
Copy link
Member

changkun commented Mar 4, 2022

Hi, thanks for the request. After a quick inspection, I think the feature is pretty much linux-only because neither windows nor darwin can get the number of readers to a given write request. Therefore I am not sure if this make sense to add to the package.

However, if you'd like to have this feature to use, I still prepared a change list, with examples showing how to use them:

diff --git a/clipboard_linux.c b/clipboard_linux.c
index 336ab2b..b487fc6 100644
--- a/clipboard_linux.c
+++ b/clipboard_linux.c
@@ -17,6 +17,7 @@
 
 // syncStatus is a function from the Go side.
 extern void syncStatus(uintptr_t handle, int status);
+extern int reachReaderLimit();
 
 void *libX11;
 
@@ -176,6 +177,10 @@ int clipboard_write(char *typ, unsigned char *buf, size_t n, uintptr_t handle) {
                 R = (*P_XChangeProperty)(ev.display, ev.requestor, ev.property,
                     XA_ATOM, 32, PropModeReplace,
                     (unsigned char *)&targets, sizeof(targets)/sizeof(Atom));
+                if (reachReaderLimit() == 1) {
+                    (*P_XCloseDisplay)(d);
+                    return 0;
+                }
             } else {
                 ev.property = None;
             }
diff --git a/clipboard_linux.go b/clipboard_linux.go
index 2137235..44b21d2 100644
--- a/clipboard_linux.go
+++ b/clipboard_linux.go
@@ -32,6 +32,7 @@ import (
 	"fmt"
 	"os"
 	"runtime"
+	"sync/atomic"
 	"time"
 	"unsafe"
 
@@ -170,3 +171,23 @@ func syncStatus(h uintptr, val int) {
 	v <- val
 	cgo.Handle(h).Delete()
 }
+
+var readLimit = int64(0)
+
+// SetReaderLimit limits the number of the readers after a Write.
+// Setting it to zero means no limit.
+func SetReaderLimit(n int) {
+	atomic.StoreInt64(&readLimit, int64(n))
+}
+
+var counter = 0
+
+//export reachReaderLimit
+func reachReaderLimit() C.int {
+	counter++
+	l := atomic.LoadInt64(&readLimit)
+	if counter%3 == 0 && l > 0 && counter/3 > int(l) {
+		return 1
+	}
+	return 0
+}

You could apply this change to a fork of this project then use the modified version.

@changkun
Copy link
Member

changkun commented Mar 4, 2022

Here is an example:

package main

import (
	"golang.design/x/clipboard"
)

func init() {
	if err := clipboard.Init(); err != nil {
		panic(err)
	}
}

func main() {
	content := "can only paste 5 times\n"

	clipboard.SetReaderLimit(5) // this

	ch := clipboard.Write(clipboard.FmtText, []byte(content))

	<-ch
}

@qjerome
Copy link
Author

qjerome commented Mar 4, 2022

This is what I would qualify as a very quick answer 👍
I'll try this out ASAP.
In my opinion, it would not be choking to have this method implemented in your package, only for Linux.
It is quite frequent that some Go packages very OS specific do not expose the same APIs on the different OS.
The simpler example that comes to my mind is the syscall package.

Thanks a lot

@changkun changkun added enhancement New feature or request platform-specific labels Mar 4, 2022
@qjerome
Copy link
Author

qjerome commented Mar 4, 2022

I tried out your patch and it seems it does not work as intended (at least in my setup).

  • When pasting to my terminal app, I can paste an infinite number of times -> it seams readReaderLimit never gets called
  • When pasting to a browser app (tested chrome and firefox), I can paste only one time (second time terminates the program)
  • When pasting to thunderbird, I can paste two times (third time terminates the program)

@changkun
Copy link
Member

changkun commented Mar 4, 2022

I see. It is probably due to application specific requests of events. The patch's approach rely on the observation that a paste can cause 3 events in the event loop (what I observed when I paste in VSCode, and why reachReaderLimit has a magic number 3).

The general idea would work but I think to really make this robust will need further investigation on how xclip resolves this..

@qjerome
Copy link
Author

qjerome commented Mar 4, 2022

It is fun because I did actually read the code ten times to understand why you did this %3 and /3.
Then I resigned and thought that it was probably a X11 related magic constant 🤣

@changkun
Copy link
Member

changkun commented Mar 4, 2022

It is fun because I did actually read the code ten times to understand why you did this %3 and /3. Then I resigned and thought that it was probably a X11 related magic constant 🤣

Sigh. I don't understand why it such a behavior. But it sounds like that an application may sent SelectionRequest multiple times. This sounds like that a reader will try to request multiple times to actually get the content, which cause multiple reads at one paste.

@qjerome
Copy link
Author

qjerome commented Mar 4, 2022

No pressure man, I'll keep an eye on your project and if you happen to find a solution for this I will use it in place of my dirty xclip hack ...
Thank you for your time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request platform-specific
Projects
None yet
Development

No branches or pull requests

2 participants