ahyatt / emacs-websocket

A websocket implementation in elisp, for emacs.
GNU General Public License v2.0
324 stars 42 forks source link

Server can't receive non-ascii text correctly #45

Closed alpha22jp closed 8 years ago

alpha22jp commented 8 years ago

29d8f2f1b8bd3d4dc452060e06ec407b0a6a8cf3 fixed sending non-ascii characters. But receiving them still appears to have a problem. I think the following test code should work, but it results

Server received text!
ws frame: "\303\244\302\275\302\240\303\245\302\245\302\275"
cl--assertion-failed: Assertion failed: (equal (car wstest-msgs) "你好")

Is it caused by the implementation of websocket.el, or am I misunderstanding the usage?

(setq wstest-closed nil)
(let ((server-conn (websocket-server
                    9998
                    :host 'local
                    :on-message (lambda (ws frame)
                                  (message "Server received text!")
                                  (websocket-send-text ws
                                                       (websocket-frame-payload frame)))
                    :on-open (lambda (_websocket) "Client connection opened!")
                    :on-close (lambda (_websocket)
                                (setq wstest-closed t)))))

  (setq wstest-msgs nil
        wstest-ws
        (websocket-open
         "ws://localhost:9998"
         :on-message (lambda (_websocket frame)
                       (push (websocket-frame-payload frame) wstest-msgs)
                       (message "ws frame: %S" (websocket-frame-payload frame)))))

  (assert (websocket-openp wstest-ws))
  (websocket-send-text wstest-ws "你好")
  (sleep-for 0.3)
  (assert (equal (car wstest-msgs) "\344\275\240\345\245\275"))
  (websocket-server-close server-conn))
(assert wstest-closed)
(websocket-close wstest-ws)
ahyatt commented 8 years ago

I can reproduce this. I think I may have just fixed the client behavior, not the server behavior. This looks a bit tricky, but I'll take a look and see if I can fix it in the next week.

alpha22jp commented 8 years ago

Hi @ahyatt,

I found the cause. It's actually a problem of test code. The string server received is UTF-8 byte sequence, so it has to be converted to unibyte format before we pass it to websocket-send-text function.

After I modified test code as below, the test passed successfully.

--- old.el        2016-10-22 10:44:21.210792558 +0900
+++ new.el        2016-10-22 10:43:56.826793474 +0900
@@ -5,7 +5,7 @@
                     :on-message (lambda (ws frame)
                                   (message "Server received text!")
                                   (websocket-send-text ws
-                                                       (websocket-frame-payload frame)))
+                                                       (decode-coding-string (string-make-unibyte (websocket-frame-payload frame)) 'utf-8)))
                     :on-open (lambda (_websocket) "Client connection opened!")
                     :on-close (lambda (_websocket)
                                 (setq wstest-closed t)))))
ahyatt commented 8 years ago

That's interesting that you solved it this way.... I looked at this yesterday and traced this down to the websocket-mask function, which incorrectly makes a string of bytes in such a way that it is interpreted as a multibyte string of what are actually unibytes. So I think working around this problem in this way may work, but shouldn't be necessary. I'll take a look tomorrow about the best way to fix it - maybe just wrapping that function in string-make-unibyte would be best.

ahyatt commented 8 years ago

Should be fixed in f7d3fb5409aed9f5cdb745f0e61a0c43c097588c, if you use the handy new function websocket-frame-text, please re-open if not.

ahyatt commented 8 years ago

That's interesting that you solved it this way.... I looked at this yesterday and traced this down to the websocket-mask function, which incorrectly makes a string of bytes in such a way that it is interpreted as a multibyte string of what are actually unibytes. So I think working around this problem in this way may work, but shouldn't be necessary. I'll take a look tomorrow about the best way to fix it.

On Fri, Oct 21, 2016 at 9:53 PM alpha22jp notifications@github.com wrote:

Hi @ahyatt https://github.com/ahyatt,

I found the cause. It's actually a problem of test code. The string server received is UTF-8 byte sequence, so it has to be converted to unibyte format before we pass it to websocket-send-text function.

After I modified test code as below, the test passed successfully.

--- old.el 2016-10-22 10:44:21.210792558 +0900+++ new.el 2016-10-22 10:43:56.826793474 +0900@@ -5,7 +5,7 @@

                 :on-message (lambda (ws frame)
                               (message "Server received text!")
                               (websocket-send-text ws
  •                                                 (websocket-frame-payload frame)))+                                                       (decode-coding-string (string-make-unibyte (websocket-frame-payload frame)) 'utf-8)))
    
               :on-open (lambda (_websocket) "Client connection opened!")
               :on-close (lambda (_websocket)
                           (setq wstest-closed t)))))

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/ahyatt/emacs-websocket/issues/45#issuecomment-255500310, or mute the thread https://github.com/notifications/unsubscribe-auth/AABP5LQpSYcnx1yjNLJw3lq5MWYZNRCsks5q2WyZgaJpZM4KaYFg .