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

Access the html5 UI through reverse proxy cause exception when websocket is connecting #2160

Closed
totaam opened this issue Feb 21, 2019 · 3 comments
Labels

Comments

@totaam
Copy link
Collaborator

totaam commented Feb 21, 2019

Issue migrated from trac ticket # 2160

component: html5 | priority: major | resolution: fixed

2019-02-21 09:32:10: tardyp created the issue


Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/xpra/net/protocol.py", line 734, in _read_parse_thread_loop
    self.do_read_parse_thread_loop()
  File "/usr/lib/python2.7/dist-packages/xpra/net/protocol.py", line 767, in do_read_parse_thread_loop
    read_buffer = read_buffer + buf
TypeError: unsupported operand type(s) for +: 'memoryview' and 'memoryview'

This bug only happens when I go through reverse proxy (traefik in kubernetes) to access the xpra websocket service.

I have a patch for that in the 2.4.x branch:

diff --git a/tags/v2.4.x/src/xpra/net/protocol.py b/tags/v2.4.x/src/xpra/net/protocol.py
index 1c4f90bda..92459108d 100644
--- a/tags/v2.4.x/src/xpra/net/protocol.py
+++ b/tags/v2.4.x/src/xpra/net/protocol.py
@@ -764,7 +764,7 @@ class Protocol(object):
                 self.idle_add(self.close)
                 return
             if read_buffer:
-                read_buffer = read_buffer + buf
+                read_buffer = memoryview_to_bytes(read_buffer) + memoryview_to_bytes(buf)
             else:
                 read_buffer = buf
             bl = len(read_buffer

This part has heavily changed in trunk, and I have not tested if this bug happens on trunk as well.

@totaam
Copy link
Collaborator Author

totaam commented Feb 21, 2019

2019-02-21 09:49:29: antoine uploaded file protocol-memoryview-merge.patch (0.6 KiB)

patch for trunk

@totaam
Copy link
Collaborator Author

totaam commented Feb 21, 2019

Following the changes in #2121, I don't think that trunk needs the patch above: my guess is that your proxy must be slicing the websocket data using continuation frames, we already handle those by calling memoryview_to_bytes since r21542.
But please test to confirm as I do not have a proxy to use for testing, otherwise 2.5 will be released soon with this bug.

As for v2.4 (soon to be abandoned), I have applied a different fix in 21784 so as not to penalize the other transport protocol backends with the extra call to memoryview_to_bytes.
Unfortunately, it is still going to make things slower for all websocket users (despite the fact that very few users may be hitting this issue..). That's one of the reasons why #2121 was needed.

Please close if that works for you.

@totaam
Copy link
Collaborator Author

totaam commented Feb 21, 2019

2019-02-21 10:32:51: tardyp commented


fixed in latest 2.5 beta

@totaam totaam closed this as completed Feb 21, 2019
@totaam totaam added the v2.4.x label Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant