xpra icon
Bug tracker and wiki

Opened 3 weeks ago

Closed 3 weeks ago

Last modified 3 weeks ago

#2129 closed defect (fixed)

File Upload with Websocket connection causing unexpected data type <type 'memoryview'>

Reported by: Mark Harkin Owned by: Mark Harkin
Priority: major Milestone: 2.5
Component: server Version: trunk
Keywords: Cc:

Description

Server: Centos7 r21499 proxy server
Client: Windows 10 r21499 python client websocket connection

Uploading a file works fine with tcp connection but
trying to upload using a websocket connection fails with server error:

2019-01-30 07:10:11,084 new proxy instance started
2019-01-30 07:10:11,084  for client ws websocket: 172.23.0.2:10000 <- 192.168.1.19:65091
2019-01-30 07:10:11,085  and server unix-domain socket:  <- /run/user/1000/xpra/890f20e343f1-0
2019-01-30 07:10:11,086 proxy instance now also available using unix domain socket:
2019-01-30 07:10:11,086  /run/user/1000/xpra/890f20e343f1-proxy-75
2019-01-30 07:10:54,966 Warning: unexpected data type <type 'memoryview'> in 'send-file-chunk' packet: <memory at 0x7f5978ed1510>
2019-01-30 07:10:54,966 failed to encode packet: ['send-file-chunk', '215a2a51217746fda5bbc72dcaef248b', 418, <memory at 0x7f5978ed1510>, True]
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/xpra/net/protocol.py", line 562, in encode
    main_packet, proto_flags = self._encoder(packet)
  File "/usr/lib/python2.7/site-packages/xpra/net/packet_encoding.py", line 89, in do_rencode
    return  rencode_dumps(data), FLAGS_RENCODE
  File "rencode/rencode.pyx", line 334, in rencode._rencode.dumps
  File "rencode/rencode.pyx", line 314, in rencode._rencode.encode
  File "rencode/rencode.pyx", line 247, in rencode._rencode.encode_list
  File "rencode/rencode.pyx", line 320, in rencode._rencode.encode
Exception: type <type 'memoryview'> not handled
2019-01-30 07:10:54,967 unsupported type: <type 'memoryview'> in 'send-file-chunk' packet->[3]
2019-01-30 07:10:54,967 Error: error in network packet write/format
2019-01-30 07:10:54,967  type <type 'memoryview'> not handled
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/xpra/net/protocol.py", line 356, in _write_format_thread_loop
    self._add_packet_to_queue(*gpc())
  File "/usr/lib/python2.7/site-packages/xpra/net/protocol.py", line 368, in _add_packet_to_queue
    chunks = self.encode(packet)
  File "/usr/lib/python2.7/site-packages/xpra/net/protocol.py", line 562, in encode
    main_packet, proto_flags = self._encoder(packet)
  File "/usr/lib/python2.7/site-packages/xpra/net/packet_encoding.py", line 89, in do_rencode
    return  rencode_dumps(data), FLAGS_RENCODE
  File "rencode/rencode.pyx", line 334, in rencode._rencode.dumps
  File "rencode/rencode.pyx", line 314, in rencode._rencode.encode
  File "rencode/rencode.pyx", line 247, in rencode._rencode.encode_list
  File "rencode/rencode.pyx", line 320, in rencode._rencode.encode
Exception: type <type 'memoryview'> not handled
2019-01-30 07:10:54,970 stopping proxy instance: server connection lost
2019-01-30 07:10:54,970 removing socket /run/user/1000/xpra/890f20e343f1-proxy-75
2019-01-30 07:10:54,971 waiting for network connections to close
2019-01-30 07:10:55,071 proxy instance 75 stopped

I feel this is similar to a previous fix r20229

Also, I'd like the client build to support websocket connections. However MINGW_BUILD.sh sets the client with --without-html5 but mimetools is required by the client to use websocket connections. Can I propose we change setup.py around line 373 from this:

if not html5_ENABLED:
    external_excludes += ["BaseHTTPServer", "mimetypes","mimetools"]

to this:

if not html5_ENABLED:
    external_excludes += ["BaseHTTPServer", "mimetypes"]
    if not client_ENABLED
        external_excludes += ["mimetools"]

Change History (8)

comment:1 Changed 3 weeks ago by Antoine Martin

Owner: changed from Antoine Martin to Mark Harkin

That's odd, the send-file-chunk packet type is already handled in ProxyInstanceProcess.process_server_packet.
Maybe we end up in Protocol.encode doing:

                    #data is small enough, inline it:
                    packet[i] = item.data
                    min_comp_size += l
                    size_check += l

Then the data would still be wrapped as a memoryview when it gets to rencode?
A fix for that would be to call memoryview_to_bytes on item.data, can you try that?
I guess this could happen if the proxy and server have different inlining thresholds - so we should probably fix that anyway.

Or maybe, it's just a case of adding send-file-chunk support in ProxyInstanceProcess.process_client_packet?

On this subject, it may be possible to avoid all this re-wrapping stuff - and make the code less brittle:

  • by making the proxy instance receive raw packets instead of cooked packets from the network layer - then it would also need to send the same raw packets out, not trivial but doable
  • by looking for memoryview attributes and re-packing them automagically

regarding the mimetools change, my instinct tells me you're correct, but what is the full error that you're seeing?
I'm in the process of completely rewriting the websocket code (#2121), so maybe this won't be needed anymore? (unlikely)

comment:2 Changed 3 weeks ago by Mark Harkin

Owner: changed from Mark Harkin to Antoine Martin

Turned out the item not the item.data was a memory view.
The below change fixed the issue:
https://github.com/mjharkin/Xpra/commit/75ad0bc461dd4273779d46efb9d4cf1af9accaa0

Can you apply this?


RE: mimetools
The error is:

xpra initialization error:
the websocket client module cannot be loaded: No module named mimetools

Missed colon in the patch above, should have been:

if not html5_ENABLED:
    external_excludes += ["BaseHTTPServer", "mimetypes"]
    if not client_ENABLED:
        external_excludes += ["mimetools"]

As you said I'd expect it to still be needed after your changes.

comment:3 Changed 3 weeks ago by Antoine Martin

Owner: changed from Antoine Martin to Mark Harkin

Turned out the item not the item.data was a memory view.

Yes, that's what this means: Exception: type <type 'memoryview'> not handled

Can you apply this?

I would rather not.
Although it will work, that's the wrong place for fixing things as this will be making copies of the packets every time.
Try r21501 instead.

comment:4 Changed 3 weeks ago by Antoine Martin

I've merged the mimetools change in r21502 - I will need to change all this as part of #2121 soon.

comment:5 Changed 3 weeks ago by Mark Harkin

Resolution: fixed
Status: newclosed

Confirmed r21501 fixes the issue. Thanks.

Looking forward to #2121.

comment:6 Changed 3 weeks ago by Antoine Martin

Thanks.

BTW, I see that you repo contains lots of changes, you might want to submit pull requests for the changes you care about before we refactor the xpra codebase, or you will have to do the work again after the refactoring, or you may have to stay behind on the version you are currently on (which will eventually break..)

comment:7 Changed 3 weeks ago by Mark Harkin

Agreed, there's at least a few changes I'd like to submit.
But there's also quite a few hacks that I know you won't and shouldn't accept.

I'll try and get some pull requests together.
When is the refactor going to happen and what's changing?

comment:8 Changed 3 weeks ago by Antoine Martin

But there's also quite a few hacks that I know you won't and shouldn't accept.

Being aware of them wouldn't hurt. In the past, we've kept some functions / attributes stable to help with external code.

When is the refactor going to happen and what's changing?

Hopefully soon: if not this cycle then the next one.
Everything will change if we move to typescript. At the very least, the codebase will be refreshed and things moved around a bit.

Note: See TracTickets for help on using tickets.